Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sync: Update attachment loadOp vs layout transition message #9539

Merged
merged 1 commit into from
Feb 26, 2025
Merged

sync: Update attachment loadOp vs layout transition message #9539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 41 additions & 37 deletions layers/sync/sync_error_messages.cpp
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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