Skip to content

Commit

Permalink
sync: Reuse image barrier message for event validation
Browse files Browse the repository at this point in the history
  • Loading branch information
@artem-lunarg
artem-lunarg committed Feb 28, 2025
1 parent 5b94e76 commit 64710ca
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 67 deletions.
7 changes: 3 additions & 4 deletions layers/sync/sync_commandbuffer.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,14 @@ class CommandExecutionContext {
virtual const SyncEventsContext *GetCurrentEventsContext() const = 0;
virtual QueueId GetQueueId() const = 0;
virtual VulkanTypedHandle Handle() const = 0;
virtual ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const = 0;
virtual std::string FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const = 0;
virtual void AddUsageRecordExtraProperties(ResourceUsageTag tag, ReportKeyValues &extra_properties) const = 0;

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

protected:
const SyncValidator &sync_state_;
Expand Down Expand Up @@ -265,7 +267,7 @@ class CommandBufferAccessContext : public CommandExecutionContext, DebugNameProv

void Reset();

ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const;
ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const override;
std::string FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const override;
void AddUsageRecordExtraProperties(ResourceUsageTag tag, ReportKeyValues &extra_properties) const override;
std::string FormatUsage(const char *usage_string, const ResourceFirstAccess &access,
Expand Down Expand Up @@ -306,9 +308,6 @@ 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; }
size_t GetTagCount() const { return access_log_->size(); }
VulkanTypedHandle Handle() const override {
Expand Down
69 changes: 22 additions & 47 deletions layers/sync/sync_error_messages.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,9 @@ static const char* string_SyncHazard(SyncHazard hazard) {
// by the synchronization. If applied synchronization covers at least stage or access component
// then we can provide more precise message by focusing on the other component.
static std::pair<bool, bool> GetPartialProtectedInfo(const SyncAccessInfo& access, const SyncAccessFlags& write_barriers,
const CommandBufferAccessContext& cb_context) {
const auto protected_stage_access_pairs =
ConvertSyncAccessesToCompactVkForm(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
cb_context.GetSyncState().extensions);
const CommandExecutionContext& context) {
const auto protected_stage_access_pairs = ConvertSyncAccessesToCompactVkForm(
write_barriers, context.GetQueueFlags(), context.GetSyncState().enabled_features, context.GetSyncState().extensions);
bool is_stage_protected = false;
bool is_access_protected = false;
for (const auto& protected_stage_access : protected_stage_access_pairs) {
Expand All @@ -94,7 +93,7 @@ static std::pair<bool, bool> GetPartialProtectedInfo(const SyncAccessInfo& acces
}

static void FormatCommonMessage(const HazardResult& hazard, const std::string& resouce_description, const vvl::Func command,
const ReportKeyValues& key_values, const CommandBufferAccessContext& cb_context,
const ReportKeyValues& key_values, const CommandExecutionContext& context,
const syncval::AdditionalMessageInfo& additional_info, std::stringstream& ss) {
const SyncHazard hazard_type = hazard.Hazard();
const SyncHazardInfo hazard_info = GetSyncHazardInfo(hazard_type);
Expand Down Expand Up @@ -138,7 +137,7 @@ static void FormatCommonMessage(const HazardResult& hazard, const std::string& r
// Invalid tag for prior access means the same command performed ILT before loadOp access
ss << "the same command";
} else {
const ReportUsageInfo usage_info = cb_context.GetReportUsageInfo(hazard.TagEx());
const ReportUsageInfo usage_info = context.GetReportUsageInfo(hazard.TagEx());
if (usage_info.command == command) {
ss << "another " << vvl::String(usage_info.command) << " command";
} else {
Expand Down Expand Up @@ -197,9 +196,9 @@ static void FormatCommonMessage(const HazardResult& hazard, const std::string& r
ss << ".";
} else if (hazard_info.IsPriorWrite()) { // RAW/WAW hazards
ss << "The current synchronization allows ";
ss << FormatSyncAccesses(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
cb_context.GetSyncState().extensions, false);
auto [is_stage_protected, is_access_protected] = GetPartialProtectedInfo(access, write_barriers, cb_context);
ss << FormatSyncAccesses(write_barriers, context.GetQueueFlags(), context.GetSyncState().enabled_features,
context.GetSyncState().extensions, false);
auto [is_stage_protected, is_access_protected] = GetPartialProtectedInfo(access, write_barriers, context);
if (is_access_protected) {
ss << " but not at " << string_VkPipelineStageFlagBits2(access.stage_mask) << ".";
} else {
Expand Down Expand Up @@ -234,27 +233,23 @@ ErrorMessages::ErrorMessages(vvl::Device& validator)
extra_properties_(validator_.syncval_settings.message_extra_properties),
pretty_print_extra_(validator_.syncval_settings.message_extra_properties_pretty_print) {}

void ErrorMessages::AddCbContextExtraProperties(const CommandBufferAccessContext& cb_context, ResourceUsageTag tag,
ReportKeyValues& key_values) const {
if (validator_.syncval_settings.message_extra_properties) {
cb_context.AddUsageRecordExtraProperties(tag, key_values);
}
}

std::string ErrorMessages::Error(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
std::string ErrorMessages::Error(const HazardResult& hazard, const CommandExecutionContext& context, vvl::Func command,
const std::string& resouce_description, const AdditionalMessageInfo& additional_info) const {
ReportKeyValues key_values;
cb_context.FormatHazard(hazard, key_values);
context.FormatHazard(hazard, key_values);
key_values.Add(kPropertyMessageType, "GeneralError");
key_values.Add(kPropertyHazardType, string_SyncHazard(hazard.Hazard()));
key_values.Add(kPropertyCommand, vvl::String(command));
for (const auto& kv : additional_info.properties.key_values) {
key_values.Add(kv.key, kv.value);
}
AddCbContextExtraProperties(cb_context, hazard.Tag(), key_values);

if (validator_.syncval_settings.message_extra_properties) {
context.AddUsageRecordExtraProperties(hazard.Tag(), key_values);
}

std::stringstream ss;
FormatCommonMessage(hazard, resouce_description, command, key_values, cb_context, additional_info, ss);
FormatCommonMessage(hazard, resouce_description, command, key_values, context, additional_info, ss);

if (!additional_info.message_end_text.empty()) {
ss << " " << additional_info.message_end_text;
Expand Down Expand Up @@ -570,47 +565,27 @@ std::string ErrorMessages::RenderPassFinalLayoutTransitionVsStoreOrResolveError(
return Error(hazard, cb_context, command, resource_description, additional_info);
}

std::string ErrorMessages::ImagePipelineBarrierError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
vvl::Func command, const std::string& resource_description,
const SyncImageMemoryBarrier& barrier) const {
std::string ErrorMessages::ImageBarrierError(const HazardResult& hazard, const CommandExecutionContext& context, vvl::Func command,
const std::string& resource_description, const SyncImageMemoryBarrier& barrier) const {
AdditionalMessageInfo additional_info;
additional_info.access_action = "performs image layout transition on the";

std::stringstream ss;
ss << "\npImageMemoryBarriers[" << barrier.barrier_index << "]: {\n";
ss << " source accesses = "
<< FormatSyncAccesses(barrier.barrier.src_access_scope, cb_context.GetQueueFlags(),
cb_context.GetSyncState().enabled_features, cb_context.GetSyncState().extensions, false)
<< FormatSyncAccesses(barrier.barrier.src_access_scope, context.GetQueueFlags(), context.GetSyncState().enabled_features,
context.GetSyncState().extensions, false)
<< ",\n";
ss << " destination accesses = "
<< FormatSyncAccesses(barrier.barrier.dst_access_scope, cb_context.GetQueueFlags(),
cb_context.GetSyncState().enabled_features, cb_context.GetSyncState().extensions, false)
<< FormatSyncAccesses(barrier.barrier.dst_access_scope, context.GetQueueFlags(), context.GetSyncState().enabled_features,
context.GetSyncState().extensions, false)
<< ",\n";
ss << " srcStageMask = " << string_VkPipelineStageFlags2(barrier.barrier.src_exec_scope.mask_param) << ",\n";
ss << " dstStageMask = " << string_VkPipelineStageFlags2(barrier.barrier.dst_exec_scope.mask_param) << ",\n";
ss << "}\n";
additional_info.message_end_text = ss.str();

return Error(hazard, cb_context, command, resource_description, additional_info);
}

std::string ErrorMessages::WaitEventsError(const HazardResult& hazard, const CommandExecutionContext& exec_context,
uint32_t image_barrier_index, const vvl::Image& image, vvl::Func command) const {
const auto format = "Hazard %s for image barrier %" PRIu32 " %s. Access info %s.";
ReportKeyValues key_values;

const std::string access_info = exec_context.FormatHazard(hazard, key_values);
std::string message = Format(format, string_SyncHazard(hazard.Hazard()), image_barrier_index,
validator_.FormatHandle(image.Handle()).c_str(), access_info.c_str());

if (extra_properties_) {
key_values.Add(kPropertyMessageType, "WaitEventsError");
key_values.Add(kPropertyHazardType, string_SyncHazard(hazard.Hazard()));
key_values.Add(kPropertyCommand, vvl::String(command));
exec_context.AddUsageRecordExtraProperties(hazard.Tag(), key_values);
message += key_values.GetExtraPropertiesSection(pretty_print_extra_);
}
return message;
return Error(hazard, context, command, resource_description, additional_info);
}

std::string ErrorMessages::FirstUseError(const HazardResult& hazard, const CommandExecutionContext& exec_context,
Expand Down
14 changes: 3 additions & 11 deletions layers/sync/sync_error_messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class ErrorMessages {
public:
explicit ErrorMessages(vvl::Device& validator);

std::string Error(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
std::string Error(const HazardResult& hazard, const CommandExecutionContext& context, vvl::Func command,
const std::string& resouce_description, const AdditionalMessageInfo& additional_info = {}) const;

std::string BufferError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
Expand Down Expand Up @@ -131,12 +131,8 @@ class ErrorMessages {
VkImageLayout old_layout, VkImageLayout new_layout,
uint32_t store_resolve_subpass) const;

std::string ImagePipelineBarrierError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
vvl::Func command, const std::string& resource_description,
const SyncImageMemoryBarrier& barrier) const;

std::string WaitEventsError(const HazardResult& hazard, const CommandExecutionContext& exec_context,
uint32_t image_barrier_index, const vvl::Image& image, vvl::Func command) const;
std::string ImageBarrierError(const HazardResult& hazard, const CommandExecutionContext& context, vvl::Func command,
const std::string& resource_description, const SyncImageMemoryBarrier& barrier) const;

std::string FirstUseError(const HazardResult& hazard, const CommandExecutionContext& exec_context,
const CommandBufferAccessContext& recorded_context, uint32_t command_buffer_index,
Expand All @@ -146,10 +142,6 @@ class ErrorMessages {
const VulkanTypedHandle& swapchain_handle, uint32_t image_index, const VulkanTypedHandle& image_handle,
vvl::Func command) const;

private:
void AddCbContextExtraProperties(const CommandBufferAccessContext& cb_context, ResourceUsageTag tag,
ReportKeyValues& key_values) const;

private:
vvl::Device& validator_;
const bool& extra_properties_;
Expand Down
12 changes: 7 additions & 5 deletions layers/sync/sync_op.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -405,10 +405,10 @@ bool SyncOpPipelineBarrier::Validate(const CommandBufferAccessContext &cb_contex
if (hazard.IsHazard()) {
LogObjectList objlist(cb_context.GetCBState().Handle(), image_state.Handle());
const Location loc(command_);
const auto &sync_state = cb_context.GetSyncState();
const SyncValidator &sync_state = cb_context.GetSyncState();
const std::string resource_description = sync_state.FormatHandle(image_state.Handle());
const std::string error = sync_state.error_messages_.ImagePipelineBarrierError(hazard, cb_context, command_,
resource_description, image_barrier);
const std::string error =
sync_state.error_messages_.ImageBarrierError(hazard, cb_context, command_, resource_description, image_barrier);
skip |= sync_state.SyncError(hazard.Hazard(), objlist, loc, error);
}
}
Expand Down Expand Up @@ -639,8 +639,10 @@ bool SyncOpWaitEvents::DoValidate(const CommandExecutionContext &exec_context, c
*image_state, subresource_range, sync_event->scope.exec_scope, src_access_scope, queue_id,
sync_event->FirstScope(), sync_event->first_scope_tag, AccessContext::DetectOptions::kDetectAll);
if (hazard.IsHazard()) {
const auto error = sync_state.error_messages_.WaitEventsError(
hazard, exec_context, image_memory_barrier.barrier_index, *image_state, command_);
LogObjectList objlist(exec_context.Handle(), image_state->Handle());
const std::string resource_description = sync_state.FormatHandle(image_state->Handle());
const std::string error = sync_state.error_messages_.ImageBarrierError(
hazard, exec_context, command_, resource_description, image_memory_barrier);
skip |= sync_state.SyncError(hazard.Hazard(), image_state->Handle(), loc, error);
break;
}
Expand Down
15 changes: 15 additions & 0 deletions layers/sync/sync_reporting.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,21 @@ void CommandBufferAccessContext::AddUsageRecordExtraProperties(ResourceUsageTag
extra_properties.Add(kPropertyResetNo, record.reset_count);
}

ReportUsageInfo QueueBatchContext::GetReportUsageInfo(ResourceUsageTagEx tag_ex) const {
BatchAccessLog::AccessRecord access = batch_log_.GetAccessRecord(tag_ex.tag);
if (!access.IsValid()) {
return {};
}
const ResourceUsageRecord &record = *access.record;
ReportUsageInfo info;
if (record.alt_usage) {
info.command = record.alt_usage.GetCommand();
} else {
info.command = record.command;
}
return info;
}

std::string QueueBatchContext::FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const {
std::stringstream out;
BatchAccessLog::AccessRecord access = batch_log_.GetAccessRecord(tag_ex.tag);
Expand Down
1 change: 1 addition & 0 deletions layers/sync/sync_submit.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ class QueueBatchContext : public CommandExecutionContext, public std::enable_sha
~QueueBatchContext();
void Trim();

ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const override;
std::string FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const override;
void AddUsageRecordExtraProperties(ResourceUsageTag tag, ReportKeyValues &extra_properties) const override;
AccessContext *GetCurrentAccessContext() override { return current_access_context_; }
Expand Down

0 comments on commit 64710ca

Please sign in to comment.