sync: Convert NegativeSyncVal.BufferCopy errors to new style

layers/sync/sync_commandbuffer.cpp

@@ -146,11 +146,12 @@ void CommandBufferAccessContext::Reset() {
     dynamic_rendering_info_.reset();
 }
 
-std::string CommandBufferAccessContext::FormatUsage(const char *usage_string, const ResourceFirstAccess &access) const {
+std::string CommandBufferAccessContext::FormatUsage(const char *usage_string, const ResourceFirstAccess &access,
+                                                    ReportKeyValues &key_values) const {
     std::stringstream out;
     assert(access.usage_info);
     out << "(" << usage_string << ": " << access.usage_info->name;
-    out << ", " << FormatUsage(access.TagEx()) << ")";
+    out << ", " << FormatUsage(access.TagEx(), key_values) << ")";
     return out.str();
 }
 

layers/sync/sync_commandbuffer.h

@@ -213,7 +213,7 @@ class CommandExecutionContext {
     virtual const SyncEventsContext *GetCurrentEventsContext() const = 0;
     virtual QueueId GetQueueId() const = 0;
     virtual VulkanTypedHandle Handle() const = 0;
-    virtual std::string FormatUsage(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;
@@ -266,10 +266,10 @@ class CommandBufferAccessContext : public CommandExecutionContext, DebugNameProv
     void Reset();
 
     ReportUsageInfo GetReportUsageInfo(ResourceUsageTagEx tag_ex) const;
-    std::string FormatUsage(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) const;  //  Only command buffers have "first usage"
+    std::string FormatUsage(const char *usage_string, const ResourceFirstAccess &access,
+                            ReportKeyValues &key_values) const;  //  Only command buffers have "first usage"
     AccessContext *GetCurrentAccessContext() override { return current_context_; }
     SyncEventsContext *GetCurrentEventsContext() override { return &events_context_; }
     const AccessContext *GetCurrentAccessContext() const override { return current_context_; }

layers/sync/sync_error_messages.cpp

@@ -120,47 +120,108 @@ std::string ErrorMessages::BufferRegionError(const HazardResult& hazard, VkBuffe
                                              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 VkPipelineStageFlags2 read_barriers = hazard.State().access_state->GetReadBarriers(hazard.State().prior_access_index);
     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 auto& sync_prior_access = syncAccessInfoByAccessIndex()[hazard.State().prior_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 ";
+    ReportKeyValues key_values;
+    cb_context.FormatHazard(hazard, key_values);
+    key_values.Add(kPropertyMessageType, "BufferRegionError");
+    const char* resource_parameter = is_src_buffer ? "srcBuffer" : "dstBuffer";
+    key_values.Add(kPropertyResourceParameter, resource_parameter);
+    AddCbContextExtraProperties(cb_context, hazard.Tag(), key_values);
+
+    const ReportUsageInfo usage_info = cb_context.GetReportUsageInfo(hazard.TagEx());
+    const SyncHazard hazard_type = hazard.Hazard();
+
+    const bool missing_synchronization =
+        ((hazard_type == READ_AFTER_WRITE || hazard_type == WRITE_AFTER_WRITE) && write_barriers.none()) ||
+        ((hazard_type == WRITE_AFTER_READ) && read_barriers == VK_PIPELINE_STAGE_2_NONE);
+    const bool is_write = hazard_type == WRITE_AFTER_WRITE || hazard_type == WRITE_AFTER_READ;
+    const bool is_prior_write = hazard_type == WRITE_AFTER_WRITE || hazard_type == READ_AFTER_WRITE;
+    const char* access_type = is_write ? "write" : "read";
+    const char* prior_access_type = is_prior_write ? "write" : "read";
+
+    bool new_style = false;  // TEMP: until handle all use cases
+    std::stringstream ss;
+    ss << string_SyncHazard(hazard.Hazard()) << " hazard detected. ";
+    ss << vvl::String(command);
+    ss << (is_write ? " writes to " : " reads ");
+    ss << validator_.FormatHandle(buffer);
+    ss << ", which was previously ";
+    ss << (is_prior_write ? "written by " : "read by ");
+    if (usage_info.command == command) {
+        ss << "another ";
+    }
+    ss << vvl::String(usage_info.command) << " command";
+    if (const auto* debug_region = key_values.FindProperty("debug_region")) {
+        ss << " (debug region: " << *debug_region << ")";
+    }
+    ss << ". ";
+
+    if (missing_synchronization) {
+        ss << "No sufficient synchronization is present to ensure that a " << access_type << " (";
+        ss << string_VkAccessFlagBits2(sync_access.access_mask) << ") at the ";
+        ss << string_VkPipelineStageFlagBits2(sync_access.stage_mask) << " stage does not conflict with a prior ";
+        ss << prior_access_type << " (" << string_VkAccessFlags2(sync_prior_access.access_mask) << ") at the ";
+        if (sync_prior_access.stage_mask == sync_access.stage_mask) {
+            ss << "same";
+        } else {
+            ss << string_VkPipelineStageFlagBits2(sync_prior_access.stage_mask);
         }
-        ss << vvl::String(usage_info.command) << " command. ";
-        ss << "The existed synchronization protects ";
-        ss << FormatSyncAccesses(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
-                                 cb_context.GetSyncState().extensions, false);
-
-        ss << " but not at " << string_VkPipelineStageFlagBits2(sync_access.stage_mask) << " stage.";
+        ss << " stage.";
+    }
+
+    if (hazard_type == WRITE_AFTER_WRITE) {
+        if (write_barriers.none()) {
+            new_style = true;
+        } else {
+            if (barrier_protects_access) {
+                ss << "The current synchronization protects ";
+                ss << FormatSyncAccesses(write_barriers, cb_context.GetQueueFlags(), cb_context.GetSyncState().enabled_features,
+                                         cb_context.GetSyncState().extensions, false);
+                ss << " but not at " << string_VkPipelineStageFlagBits2(sync_access.stage_mask) << " stage.";
+                new_style = true;
+            }
+        }
+    } else if (hazard_type == WRITE_AFTER_READ) {
+        if (!read_barriers) {
+            ss << " An execution dependency is sufficient to prevent this hazard.";
+            new_style = true;
+        }
+    } else if (hazard_type == READ_AFTER_WRITE) {
+        if (write_barriers.none()) {
+            new_style = true;
+        }
+    }
+    if (new_style) {
+        std::string message = ss.str();
+        if (extra_properties_) {
+            message += key_values.GetExtraPropertiesSection(pretty_print_extra_);
+        }
+        return message;
+    }
 
-        return ss.str();
-    } else {
+    // TEMP: Old-style formatting fallback until we handle all cases
+    {
         const auto format = "Hazard %s for %s %s, region %" PRIu32 ". Access info %s.";
-        ReportKeyValues key_values;
+        ReportKeyValues key_values2;
 
-        const std::string access_info = cb_context.FormatHazard(hazard, key_values);
-        const char* resource_parameter = is_src_buffer ? "srcBuffer" : "dstBuffer";
+        const std::string access_info = cb_context.FormatHazard(hazard, key_values2);
         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_);
+            key_values2.Add(kPropertyMessageType, "BufferRegionError");
+            key_values2.Add(kPropertyResourceParameter, resource_parameter);
+            AddCbContextExtraProperties(cb_context, hazard.Tag(), key_values2);
+            message += key_values2.GetExtraPropertiesSection(pretty_print_extra_);
         }
         return message;
     }
@@ -460,10 +521,11 @@ std::string ErrorMessages::FirstUseError(const HazardResult& hazard, const Comma
     ReportKeyValues key_values;
 
     const std::string access_info = exec_context.FormatHazard(hazard, key_values);
-    std::string message = Format(
-        format, string_SyncHazard(hazard.Hazard()), command_buffer_index, validator_.FormatHandle(recorded_handle).c_str(),
-        exec_context.ExecutionTypeString(),
-        recorded_context.FormatUsage(exec_context.ExecutionUsageString(), *hazard.RecordedAccess()).c_str(), access_info.c_str());
+    std::string message =
+        Format(format, string_SyncHazard(hazard.Hazard()), command_buffer_index, validator_.FormatHandle(recorded_handle).c_str(),
+               exec_context.ExecutionTypeString(),
+               recorded_context.FormatUsage(exec_context.ExecutionUsageString(), *hazard.RecordedAccess(), key_values).c_str(),
+               access_info.c_str());
 
     if (extra_properties_) {
         key_values.Add(kPropertyMessageType, "SubmitTimeError");

layers/sync/sync_reporting.cpp

@@ -114,6 +114,15 @@ std::string ReportKeyValues::GetExtraPropertiesSection(bool pretty_print) const
     return ss.str();
 }
 
+const std::string *ReportKeyValues::FindProperty(const std::string &key) const {
+    for (const auto &property : key_values) {
+        if (property.key == key) {
+            return &property.value;
+        }
+    }
+    return nullptr;
+}
+
 static std::string FormatHandleRecord(const HandleRecord::FormatterState &formatter) {
     std::stringstream out;
     const HandleRecord &handle = formatter.that;
@@ -143,7 +152,7 @@ static std::string FormatHandleRecord(const HandleRecord::FormatterState &format
     return out.str();
 }
 
-static std::string FormatResourceUsageRecord(const ResourceUsageRecord::FormatterState &formatter) {
+static std::string FormatResourceUsageRecord(const ResourceUsageRecord::FormatterState &formatter, ReportKeyValues &key_values) {
     std::stringstream out;
     const ResourceUsageRecord &record = formatter.record;
     if (record.alt_usage) {
@@ -173,6 +182,7 @@ static std::string FormatResourceUsageRecord(const ResourceUsageRecord::Formatte
             const std::string debug_region_name = formatter.debug_name_provider->GetDebugRegionName(record);
             if (!debug_region_name.empty()) {
                 out << ", debug_region: " << debug_region_name;
+                key_values.Add(kPropertyDebugRegion, debug_region_name);
             }
         }
     }
@@ -408,7 +418,7 @@ std::string CommandExecutionContext::FormatHazard(const HazardResult &hazard, Re
     std::stringstream out;
     assert(hazard.IsHazard());
     out << FormatHazardState(hazard.State(), queue_flags_, sync_state_.enabled_features, sync_state_.extensions, key_values);
-    out << ", " << FormatUsage(hazard.TagEx()) << ")";
+    out << ", " << FormatUsage(hazard.TagEx(), key_values) << ")";
     return out.str();
 }
 
@@ -423,14 +433,15 @@ ReportUsageInfo CommandBufferAccessContext::GetReportUsageInfo(ResourceUsageTagE
     return info;
 }
 
-std::string CommandBufferAccessContext::FormatUsage(ResourceUsageTagEx tag_ex) const {
+std::string CommandBufferAccessContext::FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const {
     if (tag_ex.tag >= access_log_->size()) return std::string();
 
     std::stringstream out;
     assert(tag_ex.tag < access_log_->size());
     const auto &record = (*access_log_)[tag_ex.tag];
     const auto debug_name_provider = (record.label_command_index == vvl::kU32Max) ? nullptr : this;
-    out << FormatResourceUsageRecord(record.Formatter(sync_state_, cb_state_, debug_name_provider, tag_ex.handle_index));
+    out << FormatResourceUsageRecord(record.Formatter(sync_state_, cb_state_, debug_name_provider, tag_ex.handle_index),
+                                     extra_properties);
     return out.str();
 }
 
@@ -444,7 +455,7 @@ void CommandBufferAccessContext::AddUsageRecordExtraProperties(ResourceUsageTag
     extra_properties.Add(kPropertyResetNo, record.reset_count);
 }
 
-std::string QueueBatchContext::FormatUsage(ResourceUsageTagEx tag_ex) const {
+std::string QueueBatchContext::FormatUsage(ResourceUsageTagEx tag_ex, ReportKeyValues &extra_properties) const {
     std::stringstream out;
     BatchAccessLog::AccessRecord access = batch_log_.GetAccessRecord(tag_ex.tag);
     if (access.IsValid()) {
@@ -455,7 +466,8 @@ std::string QueueBatchContext::FormatUsage(ResourceUsageTagEx tag_ex) const {
             out << FormatStateObject(SyncNodeFormatter(sync_state_, batch.queue->GetQueueState()));
             out << ", submit: " << batch.submit_index << ", batch: " << batch.batch_index << ", ";
         }
-        out << FormatResourceUsageRecord(record.Formatter(sync_state_, nullptr, access.debug_name_provider, tag_ex.handle_index));
+        out << FormatResourceUsageRecord(record.Formatter(sync_state_, nullptr, access.debug_name_provider, tag_ex.handle_index),
+                                         extra_properties);
     }
     return out.str();
 }

layers/sync/sync_reporting.h

@@ -54,6 +54,7 @@ struct ReportKeyValues {
     void Add(std::string_view key, uint64_t value);
 
     std::string GetExtraPropertiesSection(bool pretty_print) const;
+    const std::string* FindProperty(const std::string &key) const;
 };
 
 struct ReportUsageInfo {
@@ -73,6 +74,7 @@ inline constexpr const char *kPropertyAccess = "access";
 inline constexpr const char *kPropertyPriorAccess = "prior_access";
 inline constexpr const char *kPropertyReadBarriers = "read_barriers";
 inline constexpr const char *kPropertyWriteBarriers = "write_barriers";
+inline constexpr const char *kPropertyDebugRegion = "debug_region";
 inline constexpr const char *kPropertyLoadOp = "load_op";
 inline constexpr const char *kPropertyStoreOp = "store_op";
 inline constexpr const char *kPropertyResolveMode = "resolve_mode";

layers/sync/sync_submit.h

@@ -295,7 +295,7 @@ class QueueBatchContext : public CommandExecutionContext, public std::enable_sha
     ~QueueBatchContext();
     void Trim();
 
-    std::string FormatUsage(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_; }
     const AccessContext *GetCurrentAccessContext() const override { return current_access_context_; }