Skip to content

Commit

Permalink
sync: Add example of new style of syncval reporting
Browse files Browse the repository at this point in the history
This demonstrates new style of synval error messages. Only one specifc
usage case is updated. The following work will involve converting other
messages and updating the reporting helpers.
  • Loading branch information
artem-lunarg committed Jan 29, 2025
1 parent 336e551 commit fa49f6f
Show file tree
Hide file tree
Showing 9 changed files with 169 additions and 56 deletions.
10 changes: 7 additions & 3 deletions layers/sync/sync_commandbuffer.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2019-2024 Valve Corporation
* Copyright (c) 2019-2024 LunarG, Inc.
* Copyright (c) 2019-2025 Valve Corporation
* Copyright (c) 2019-2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,6 +17,7 @@
#pragma once

#include "sync/sync_renderpass.h"
#include "sync/sync_reporting.h"
#include "state_tracker/cmd_buffer_state.h"

struct ReportKeyValues;
Expand All @@ -33,6 +34,7 @@ class AlternateResourceUsage {
using Record = std::unique_ptr<RecordBase>;
virtual Record MakeRecord() const = 0;
virtual std::ostream &Format(std::ostream &out, const SyncValidator &sync_state) const = 0;
virtual vvl::Func GetCommand() const = 0;
virtual ~RecordBase() {}
};

Expand All @@ -46,6 +48,7 @@ class AlternateResourceUsage {
FormatterState Formatter(const SyncValidator &sync_state) const { return FormatterState(sync_state, *this); };

std::ostream &Format(std::ostream &out, const SyncValidator &sync_state) const { return record_->Format(out, sync_state); };
vvl::Func GetCommand() const { return record_->GetCommand(); }
AlternateResourceUsage() = default;
AlternateResourceUsage(const RecordBase &record) : record_(record.MakeRecord()) {}
AlternateResourceUsage(const AlternateResourceUsage &other) : record_() {
Expand Down Expand Up @@ -214,7 +217,6 @@ class CommandExecutionContext {
virtual void AddUsageRecordExtraProperties(ResourceUsageTag tag, ReportKeyValues &extra_properties) const = 0;

std::string FormatHazard(const HazardResult &hazard, ReportKeyValues &key_values) const;
std::string FormatHazard(const HazardResult &hazard) const;
bool ValidForSyncOps() const;
const SyncValidator &GetSyncState() const { return sync_state_; }

Expand Down Expand Up @@ -263,6 +265,7 @@ class CommandBufferAccessContext : public CommandExecutionContext, DebugNameProv

void Reset();

ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const;
std::string FormatUsage(ResourceUsageTagEx tag_ex) const override;
void AddUsageRecordExtraProperties(ResourceUsageTag tag, ReportKeyValues &extra_properties) const override;
std::string FormatUsage(const char *usage_string,
Expand Down Expand Up @@ -303,6 +306,7 @@ class CommandBufferAccessContext : public CommandExecutionContext, DebugNameProv
void RecordExecutedCommandBuffer(const CommandBufferAccessContext &recorded_context);
void ResolveExecutedCommandBuffer(const AccessContext &recorded_context, ResourceUsageTag offset);

// TODO: what about using queue_flags directly from base class?
VkQueueFlags GetQueueFlags() const { return cb_state_ ? cb_state_->GetQueueFlags() : 0; }

ExecutionType Type() const override { return kExecuted; }
Expand Down
65 changes: 49 additions & 16 deletions layers/sync/sync_error_messages.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Copyright (c) 2024 The Khronos Group Inc.
* Copyright (c) 2024 Valve Corporation
* Copyright (c) 2024 LunarG, Inc.
/* Copyright (c) 2025 The Khronos Group Inc.
* Copyright (c) 2025 Valve Corporation
* Copyright (c) 2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -19,6 +19,7 @@
#include "sync/sync_commandbuffer.h"
#include "sync/sync_image.h"
#include "sync/sync_reporting.h"
#include "sync/sync_validation.h"
#include "state_tracker/buffer_state.h"
#include "state_tracker/descriptor_sets.h"

Expand Down Expand Up @@ -148,21 +149,53 @@ std::string ErrorMessages::BufferError(const HazardResult& hazard, VkBuffer buff
}

std::string ErrorMessages::BufferRegionError(const HazardResult& hazard, VkBuffer buffer, bool is_src_buffer, uint32_t region_index,
const CommandBufferAccessContext& cb_context) const {
const auto format = "Hazard %s for %s %s, region %" PRIu32 ". Access info %s.";
ReportKeyValues key_values;
const CommandBufferAccessContext& cb_context, const vvl::Func command) const {
// TEMP: will be part of more general code
const SyncAccessFlags write_barriers = hazard.State().access_state->GetWriteBarriers();
const auto vk_protected_accesses =
ConvertSyncAccessesToCompactVkForm(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
cb_context.GetSyncState().extensions);
const auto& sync_access = syncAccessInfoByAccessIndex()[hazard.State().access_index];
const bool barrier_protects_access =
std::find_if(vk_protected_accesses.begin(), vk_protected_accesses.end(), [&sync_access](const auto& protected_access) {
return (protected_access.second & sync_access.access_mask) != 0;
}) != vk_protected_accesses.end();

// TEMP: detect specific case to demo new direction of syncval error messages.
// This also will be replaced by more general implementation.
if (hazard.Hazard() == WRITE_AFTER_WRITE && !write_barriers.none() && barrier_protects_access) {
const ReportUsageInfo usage_info = cb_context.GetReportUsageInfo(hazard.TagEx());
std::stringstream ss;
ss << string_SyncHazard(hazard.Hazard()) << " hazard detected. ";
ss << vvl::String(command) << " writes to " << validator_.FormatHandle(buffer);
ss << ", which was written earlier by ";
if (usage_info.command == command) {
ss << "another ";
}
ss << vvl::String(usage_info.command) << ". ";
ss << "The applied synchronization protects ";
ss << FormatSyncAccesses(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
cb_context.GetSyncState().extensions, false);

ss << " but does not cover accesses at " << string_VkPipelineStageFlagBits2(sync_access.stage_mask) << " stage.";

return ss.str();
} else {
const auto format = "Hazard %s for %s %s, region %" PRIu32 ". Access info %s.";
ReportKeyValues key_values;

const std::string access_info = cb_context.FormatHazard(hazard, key_values);
const char* resource_parameter = is_src_buffer ? "srcBuffer" : "dstBuffer";
std::string message = Format(format, string_SyncHazard(hazard.Hazard()), resource_parameter,
validator_.FormatHandle(buffer).c_str(), region_index, access_info.c_str());
if (extra_properties_) {
key_values.Add(kPropertyMessageType, "BufferRegionError");
key_values.Add(kPropertyResourceParameter, resource_parameter);
AddCbContextExtraProperties(cb_context, hazard.Tag(), key_values);
message += key_values.GetExtraPropertiesSection(pretty_print_extra_);
const std::string access_info = cb_context.FormatHazard(hazard, key_values);
const char* resource_parameter = is_src_buffer ? "srcBuffer" : "dstBuffer";
std::string message = Format(format, string_SyncHazard(hazard.Hazard()), resource_parameter,
validator_.FormatHandle(buffer).c_str(), region_index, access_info.c_str());
if (extra_properties_) {
key_values.Add(kPropertyMessageType, "BufferRegionError");
key_values.Add(kPropertyResourceParameter, resource_parameter);
AddCbContextExtraProperties(cb_context, hazard.Tag(), key_values);
message += key_values.GetExtraPropertiesSection(pretty_print_extra_);
}
return message;
}
return message;
}

std::string ErrorMessages::ImageRegionError(const HazardResult& hazard, VkImage image, bool is_src_image, uint32_t region_index,
Expand Down
8 changes: 4 additions & 4 deletions layers/sync/sync_error_messages.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Copyright (c) 2024 The Khronos Group Inc.
* Copyright (c) 2024 Valve Corporation
* Copyright (c) 2024 LunarG, Inc.
/* Copyright (c) 2025 The Khronos Group Inc.
* Copyright (c) 2025 Valve Corporation
* Copyright (c) 2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -45,7 +45,7 @@ class ErrorMessages {
const CommandBufferAccessContext& cb_context) const;

std::string BufferRegionError(const HazardResult& hazard, VkBuffer buffer, bool is_src_buffer, uint32_t region_index,
const CommandBufferAccessContext& cb_context) const;
const CommandBufferAccessContext& cb_context, const vvl::Func command) const;

std::string ImageRegionError(const HazardResult& hazard, VkImage image, bool is_src_image, uint32_t region_index,
const CommandBufferAccessContext& cb_context) const;
Expand Down
45 changes: 30 additions & 15 deletions layers/sync/sync_reporting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ static std::string FormatHandleRecord(const HandleRecord::FormatterState &format
return out.str();
}

std::string FormatResourceUsageRecord(const ResourceUsageRecord::FormatterState &formatter) {
static std::string FormatResourceUsageRecord(const ResourceUsageRecord::FormatterState &formatter) {
std::stringstream out;
const ResourceUsageRecord &record = formatter.record;
if (record.alt_usage) {
Expand Down Expand Up @@ -235,11 +235,11 @@ static SyncAccessFlags FilterSyncAccessesByAllowedVkAccesses(const SyncAccessFla
return filtered_accesses;
}

static std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQueueFlags allowed_queue_flags,
const DeviceFeatures &features, const DeviceExtensions &device_extensions,
bool format_as_extra_property) {
std::vector<std::pair<VkPipelineStageFlags2, VkAccessFlags2>> ConvertSyncAccessesToCompactVkForm(
const SyncAccessFlags &sync_accesses, VkQueueFlags allowed_queue_flags, const DeviceFeatures &features,
const DeviceExtensions &device_extensions) {
if (sync_accesses.none()) {
return "0";
return {};
}

const VkPipelineStageFlags2 disabled_stages = sync_utils::DisabledPipelineStages(features, device_extensions);
Expand Down Expand Up @@ -281,7 +281,7 @@ static std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQu
}

// Replace sequences of stages/accesses with more compact equivalent meta values where possible
std::vector<std::pair<VkPipelineStageFlags2, VkAccessFlags2>> report_accesses;
std::vector<std::pair<VkPipelineStageFlags2, VkAccessFlags2>> result;
VkPipelineStageFlags2 stages_with_all_supported_accesses = 0;
VkAccessFlags2 all_accesses = 0; // accesses for the above stages

Expand Down Expand Up @@ -312,20 +312,29 @@ static std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQu

sync_utils::ReplaceExpandBitsWithMetaMask(stages, all_transfer_expand_bits, VK_PIPELINE_STAGE_2_ALL_TRANSFER_BIT);
sync_utils::ReplaceExpandBitsWithMetaMask(accesses, kShaderReadExpandBits, VK_ACCESS_2_SHADER_READ_BIT);
report_accesses.emplace_back(stages, accesses);
result.emplace_back(stages, accesses);
}
if (stages_with_all_supported_accesses) {
if (IsSingleBitSet(stages_with_all_supported_accesses) && GetBitSetCount(all_accesses) <= 2) {
// For simple configurations (1 stage and at most 2 accesses) don't use ALL accesses shortcut
report_accesses.emplace_back(stages_with_all_supported_accesses, all_accesses);
result.emplace_back(stages_with_all_supported_accesses, all_accesses);
} else {
sync_utils::ReplaceExpandBitsWithMetaMask(stages_with_all_supported_accesses, all_transfer_expand_bits,
VK_PIPELINE_STAGE_2_ALL_TRANSFER_BIT);
report_accesses.emplace_back(stages_with_all_supported_accesses, sync_utils::kAllAccesses);
result.emplace_back(stages_with_all_supported_accesses, sync_utils::kAllAccesses);
}
}
return result;
}

// Create string with available accesses
std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQueueFlags allowed_queue_flags,
const DeviceFeatures &features, const DeviceExtensions &device_extensions,
bool format_as_extra_property) {
const auto report_accesses =
ConvertSyncAccessesToCompactVkForm(sync_accesses, allowed_queue_flags, features, device_extensions);
if (report_accesses.empty()) {
return "0";
}
std::stringstream out;
bool first = true;
for (const auto &[stages, accesses] : report_accesses) {
Expand All @@ -340,9 +349,9 @@ static std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQu
}
} else {
if (accesses == sync_utils::kAllAccesses) {
out << "all accesses on " << string_VkPipelineStageFlags2(stages) << " stage";
out << "all accesses at " << string_VkPipelineStageFlags2(stages) << " stage";
} else {
out << string_VkAccessFlags2(accesses) << " accesses on " << string_VkPipelineStageFlags2(stages) << " stage";
out << string_VkAccessFlags2(accesses) << " accesses at " << string_VkPipelineStageFlags2(stages) << " stage";
}
}
first = false;
Expand Down Expand Up @@ -403,9 +412,15 @@ std::string CommandExecutionContext::FormatHazard(const HazardResult &hazard, Re
return out.str();
}

std::string CommandExecutionContext::FormatHazard(const HazardResult &hazard) const {
ReportKeyValues key_values;
return FormatHazard(hazard, key_values);
ReportUsageInfo CommandBufferAccessContext::GetReportUsageInfo(ResourceUsageTagEx tag_ex) const {
const ResourceUsageRecord &record = (*access_log_)[tag_ex.tag];
ReportUsageInfo info;
if (record.alt_usage) {
info.command = record.alt_usage.GetCommand();
} else {
info.command = record.command;
}
return info;
}

std::string CommandBufferAccessContext::FormatUsage(ResourceUsageTagEx tag_ex) const {
Expand Down
32 changes: 28 additions & 4 deletions layers/sync/sync_reporting.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Copyright (c) 2024 The Khronos Group Inc.
* Copyright (c) 2024 Valve Corporation
* Copyright (c) 2024 LunarG, Inc.
/* Copyright (c) 2025 The Khronos Group Inc.
* Copyright (c) 2025 Valve Corporation
* Copyright (c) 2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -17,7 +17,19 @@

#pragma once

#include "sync/sync_commandbuffer.h"
#include "generated/error_location_helper.h"
#include "generated/sync_validation_types.h"

namespace vvl {
class CommandBuffer;
class StateObject;
class Image;
class Queue;
}
class DebugReport;
class SyncValidator;
struct DeviceFeatures;
struct DeviceExtensions;

struct SyncNodeFormatter {
const DebugReport *debug_report;
Expand All @@ -44,6 +56,18 @@ struct ReportKeyValues {
std::string GetExtraPropertiesSection(bool pretty_print) const;
};

struct ReportUsageInfo {
vvl::Func command = vvl::Func::Empty;
};

std::vector<std::pair<VkPipelineStageFlags2, VkAccessFlags2>> ConvertSyncAccessesToCompactVkForm(
const SyncAccessFlags &sync_accesses, VkQueueFlags allowed_queue_flags, const DeviceFeatures &features,
const DeviceExtensions &device_extensions);

std::string FormatSyncAccesses(const SyncAccessFlags &sync_accesses, VkQueueFlags allowed_queue_flags,
const DeviceFeatures &features, const DeviceExtensions &device_extensions,
bool format_as_extra_property);

inline constexpr const char *kPropertyMessageType = "message_type";
inline constexpr const char *kPropertyAccess = "access";
inline constexpr const char *kPropertyPriorAccess = "prior_access";
Expand Down
6 changes: 4 additions & 2 deletions layers/sync/sync_submit.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* Copyright (c) 2019-2024 Valve Corporation
* Copyright (c) 2019-2024 LunarG, Inc.
* Copyright (c) 2019-2025 Valve Corporation
* Copyright (c) 2019-2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -265,6 +265,7 @@ class QueueBatchContext : public CommandExecutionContext, public std::enable_sha
~PresentResourceRecord() override {}
PresentResourceRecord(const PresentedImageRecord &presented) : presented_(presented) {}
std::ostream &Format(std::ostream &out, const SyncValidator &sync_state) const override;
vvl::Func GetCommand() const override { return vvl::Func::vkQueuePresentKHR; }

private:
PresentedImageRecord presented_;
Expand All @@ -277,6 +278,7 @@ class QueueBatchContext : public CommandExecutionContext, public std::enable_sha
AcquireResourceRecord(const PresentedImageRecord &presented, ResourceUsageTag tag, vvl::Func command)
: presented_(presented), acquire_tag_(tag), command_(command) {}
std::ostream &Format(std::ostream &out, const SyncValidator &sync_state) const override;
vvl::Func GetCommand() const override { return command_; }

private:
PresentedImageRecord presented_;
Expand Down
Loading

0 comments on commit fa49f6f

Please sign in to comment.