Skip to content

Commit

Permalink
sync: Update renderpass loadop vs layout transition message
Browse files Browse the repository at this point in the history
  • Loading branch information
@artem-lunarg
artem-lunarg committed Feb 26, 2025
1 parent 68535ea commit 4da1db9
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 50 deletions.
78 changes: 41 additions & 37 deletions layers/sync/sync_error_messages.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ static void FormatCommonMessage(const HazardResult& hazard, const std::string& r
const SyncHazard hazard_type = hazard.Hazard();
const SyncHazardInfo hazard_info = GetSyncHazardInfo(hazard_type);

const ReportUsageInfo usage_info = cb_context.GetReportUsageInfo(hazard.TagEx());

const SyncAccessInfo& access = syncAccessInfoByAccessIndex()[hazard.State().access_index];
const SyncAccessInfo& prior_access = syncAccessInfoByAccessIndex()[hazard.State().prior_access_index];

Expand All @@ -111,7 +109,11 @@ static void FormatCommonMessage(const HazardResult& hazard, const std::string& r
(hazard_info.IsPriorRead() && read_barriers == VK_PIPELINE_STAGE_2_NONE);

// Brief description of what happened
ss << string_SyncHazard(hazard_type) << " hazard detected. ";
ss << string_SyncHazard(hazard_type) << " hazard detected";
if (!additional_info.hazard_overview.empty()) {
ss << ": " << additional_info.hazard_overview;
}
ss << ". ";
ss << (additional_info.access_initiator.empty() ? vvl::String(command) : additional_info.access_initiator);
ss << " ";
if (!additional_info.access_action.empty()) {
Expand All @@ -130,10 +132,16 @@ static void FormatCommonMessage(const HazardResult& hazard, const std::string& r
} else {
ss << "read by ";
}
if (usage_info.command == command) {
ss << "another " << vvl::String(usage_info.command) << " command";
if (hazard.Tag() == kInvalidTag) {
// Invalid tag for prior access means the same command performed ILT before loadOp access
ss << "the same command";
} else {
ss << vvl::String(usage_info.command);
const ReportUsageInfo usage_info = cb_context.GetReportUsageInfo(hazard.TagEx());
if (usage_info.command == command) {
ss << "another " << vvl::String(usage_info.command) << " command";
} else {
ss << vvl::String(usage_info.command);
}
}
if (const auto* debug_region = key_values.FindProperty("debug_region")) {
ss << " (debug region: " << *debug_region << ")";
Expand Down Expand Up @@ -387,6 +395,20 @@ static const char* GetLoadOpActionName(VkAttachmentLoadOp load_op) {
return "";
}

static void CheckForLoadOpDontCareInsight(VkAttachmentLoadOp load_op, bool is_color, std::string& message_end_text) {
if (load_op == VK_ATTACHMENT_LOAD_OP_DONT_CARE) {
std::stringstream ss;
ss << "\nVulkan insight: according to the specification VK_ATTACHMENT_LOAD_OP_DONT_CARE is a write access (";
if (is_color) {
ss << "VK_ACCESS_2_COLOR_ATTACHMENT_WRITE_BIT for color attachment";
} else {
ss << "VK_ACCESS_2_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT for depth/stencil attachment";
}
ss << ").";
message_end_text += ss.str();
}
}

std::string ErrorMessages::BeginRenderingError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
vvl::Func command, const std::string& resource_description,
VkAttachmentLoadOp load_op) const {
Expand Down Expand Up @@ -423,18 +445,20 @@ std::string ErrorMessages::RenderPassLoadOpError(const HazardResult& hazard, con
const char* load_op_str = string_VkAttachmentLoadOp(load_op);
additional_info.properties.Add(kPropertyLoadOp, load_op_str);
additional_info.access_action = GetLoadOpActionName(load_op);
CheckForLoadOpDontCareInsight(load_op, is_color, additional_info.message_end_text);
return Error(hazard, cb_context, command, resource_description, additional_info);
}

if (load_op == VK_ATTACHMENT_LOAD_OP_DONT_CARE) {
std::stringstream ss;
ss << "\nVulkan insight: according to the specification VK_ATTACHMENT_LOAD_OP_DONT_CARE is a write access (";
if (is_color) {
ss << "VK_ACCESS_2_COLOR_ATTACHMENT_WRITE_BIT for color attachment";
} else {
ss << "VK_ACCESS_2_DEPTH_STENCIL_ATTACHMENT_WRITE_BIT for depth/stencil attachment";
}
ss << ").";
additional_info.message_end_text = ss.str();
}
std::string ErrorMessages::RenderPassLoadOpVsLayoutTransitionError(const HazardResult& hazard,
const CommandBufferAccessContext& cb_context, vvl::Func command,
const std::string& resource_description,
VkAttachmentLoadOp load_op, bool is_color) const {
AdditionalMessageInfo additional_info;
const char* load_op_str = string_VkAttachmentLoadOp(load_op);
additional_info.properties.Add(kPropertyLoadOp, load_op_str);
additional_info.hazard_overview = "attachment loadOp access is not synchronized with the attachment layout transition";
additional_info.access_action = GetLoadOpActionName(load_op);
CheckForLoadOpDontCareInsight(load_op, is_color, additional_info.message_end_text);
return Error(hazard, cb_context, command, resource_description, additional_info);
}

Expand Down Expand Up @@ -565,26 +589,6 @@ std::string ErrorMessages::RenderPassLayoutTransitionError(const HazardResult& h
return message;
}

std::string ErrorMessages::RenderPassLoadOpVsLayoutTransitionError(const HazardResult& hazard, uint32_t subpass,
uint32_t attachment, const char* aspect_name,
VkAttachmentLoadOp load_op, vvl::Func command) const {
const auto format =
"Hazard %s vs. layout transition in subpass %" PRIu32 " for attachment %" PRIu32 " aspect %s during load with loadOp %s.";

const char* load_op_str = string_VkAttachmentLoadOp(load_op);
std::string message = Format(format, string_SyncHazard(hazard.Hazard()), subpass, attachment, aspect_name, load_op_str);

if (extra_properties_) {
ReportKeyValues key_values;
key_values.Add(kPropertyMessageType, "RenderPassLoadOpVsLayoutTransitionError");
key_values.Add(kPropertyHazardType, string_SyncHazard(hazard.Hazard()));
key_values.Add(kPropertyCommand, vvl::String(command));
key_values.Add(kPropertyLoadOp, load_op_str);
message += key_values.GetExtraPropertiesSection(pretty_print_extra_);
}
return message;
}

std::string ErrorMessages::RenderPassColorAttachmentError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
const vvl::ImageView& view, uint32_t attachment,
vvl::Func command) const {
Expand Down
7 changes: 4 additions & 3 deletions layers/sync/sync_error_messages.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct AdditionalMessageInfo {
// For example, "clears" for a clear operation might be more specific than a write
std::string access_action;

std::string hazard_overview;
std::string pre_synchronization_text;
std::string message_end_text;
};
Expand Down Expand Up @@ -101,6 +102,9 @@ class ErrorMessages {
std::string RenderPassLoadOpError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
const std::string& resource_description, uint32_t subpass, uint32_t attachment,
VkAttachmentLoadOp load_op, bool is_color) const;
std::string RenderPassLoadOpVsLayoutTransitionError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
vvl::Func command, const std::string& resource_description,
VkAttachmentLoadOp load_op, bool is_color) const;
std::string RenderPassResolveError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
const std::string& resource_description) const;
std::string RenderPassStoreOpError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context, vvl::Func command,
Expand All @@ -124,9 +128,6 @@ class ErrorMessages {
uint32_t subpass, uint32_t attachment, VkImageLayout old_layout,
VkImageLayout new_layout, vvl::Func command) const;

std::string RenderPassLoadOpVsLayoutTransitionError(const HazardResult& hazard, uint32_t subpass, uint32_t attachment,
const char* aspect_name, VkAttachmentLoadOp load_op,
vvl::Func command) const;

std::string RenderPassColorAttachmentError(const HazardResult& hazard, const CommandBufferAccessContext& cb_context,
const vvl::ImageView& view, uint32_t attachment, vvl::Func command) const;
Expand Down
20 changes: 10 additions & 10 deletions layers/sync/sync_renderpass.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -235,18 +235,18 @@ bool RenderPassAccessContext::ValidateLoadOperation(const CommandBufferAccessCon
const VkAttachmentLoadOp load_op = checked_stencil ? ci.stencilLoadOp : ci.loadOp;
const auto &sync_state = cb_context.GetSyncState();
const Location loc(command);
if (hazard.Tag() == kInvalidTag) {
// Hazard vs. ILT
const auto error = sync_state.error_messages_.RenderPassLoadOpVsLayoutTransitionError(hazard, subpass, i,
aspect, load_op, command);

std::stringstream ss;
ss << "the " << aspect << " aspect of attachment " << i << " in subpass " << subpass;
ss << " (" << sync_state.FormatHandle(view_gen.GetViewState()->Handle());
ss << ", loadOp " << string_VkAttachmentLoadOp(load_op) << ")";
const std::string resource_description = ss.str();

if (hazard.Tag() == kInvalidTag) { // Hazard vs. ILT
const auto error = sync_state.error_messages_.RenderPassLoadOpVsLayoutTransitionError(
hazard, cb_context, command, resource_description, load_op, is_color);
skip |= sync_state.SyncError(hazard.Hazard(), rp_state.Handle(), loc, error);
} else {
std::stringstream ss;
ss << aspect << " aspect of attachment " << i << " in subpass " << subpass;
ss << " (" << sync_state.FormatHandle(view_gen.GetViewState()->Handle());
ss << ", loadOp " << string_VkAttachmentLoadOp(load_op) << ")";
const std::string resource_description = ss.str();

const std::string error = sync_state.error_messages_.RenderPassLoadOpError(
hazard, cb_context, command, resource_description, subpass, i, load_op, is_color);
skip |= sync_state.SyncError(hazard.Hazard(), rp_state.Handle(), loc, error);
Expand Down

0 comments on commit 4da1db9

Please sign in to comment.