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.

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

gpuav: Update debug label regions management #8886

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

arno-lunarg
Copy link
Contributor

Took some of what Artem did for syncval
Add debug label regions for debug printf
Still need to add support for secondary command buffers, not so easy

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 305756.

std::string debug_region_name;
if (!cb_state.GetLabelCommands().empty()) {
const uint32_t last_label_command_i = static_cast<uint32_t>(cb_state.GetLabelCommands().size() - 1);
debug_region_name = cb_state.GetDebugRegionName(cb_state.GetLabelCommands(), last_label_command_i);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how I use GetDebugRegionName, anything looking wrong @artem-lunarg ?

Copy link
Contributor

@artem-lunarg artem-lunarg Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Not sure what is the scope in gpu-av. If gpu-av needs also to consider global label state then GetDebugRegionName() provides initial_label_stack parameter and it can be used to add that support too.

Label commands are not limited to a single command buffer, in general. EndLabel can be in different command buffer. So after specific queue submission the queue can have current non-empty label name (per-queue state). It can be used as initial_label_stack to prepend when replaying label commands from current command buffer.

Not asking about change here (could be non-trivial change), just for awareness, in case you need this.

Copy link
Contributor

@artem-lunarg artem-lunarg Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this label stuff is really not the obvious, one example. It's fine to start command buffer with EndLabel command (I think I found this in real app too). This assumes that BeginLabel was in another command buffer. So queue-level state tracking helps with this.

NegativeSyncValReporting tests with DebugRegion in the name have various examples

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ultimately we want the same level of support as sync val, so this is valuable info! I need to look at how you do those things and will probably ping you again on this topic

Copy link
Contributor

@artem-lunarg artem-lunarg Nov 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just refreshed memory how queue label state is tracked. It's vvl::Queue and cmdbuf_label_stack field.
Example how it is used in syncval:

auto current_label_stack = queue_sync_state->GetQueueState()->cmdbuf_label_stack;

One difference of tracking queue label state comparing to command buffer label tracking is that on queue level it's enough to keep std::vector<string> of nested labels, in contrast to LabelCommand in the command buffer. The reason is because after command buffer is submitted we know exact label hierarchy (just replay label commands from entire submitted command buffer) so we just save result of this replay.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18062 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18062 failed.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 306110.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18069 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18069 failed.

Took some of what Artem did for syncval
Add debug label regions for debug printf
Still need to add support for secondary command buffers, not so easy
@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 306142.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18070 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18070 failed.

Copy link
Contributor

@artem-lunarg artem-lunarg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if CI is happy

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build queued with queue ID 306976.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18088 running.

@ci-tester-lunarg
Copy link
Collaborator

CI Vulkan-ValidationLayers build # 18088 passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants