KhronosGroup / Vulkan-ValidationLayers

Vulkan Validation Layers (VVL)
https://vulkan.lunarg.com/doc/sdk/latest/linux/khronos_validation_layer.html
Other
751 stars 403 forks source link

No resource type and name in error message #8139

Open Goshido opened 3 months ago

Goshido commented 3 months ago

Environment:

Describe the Issue

Is it normal that sync validation does not provide description which kind of resource is not synchronized? I mean VkImage or VkBuffer. I gave meaningful names to each Vulkan resource in the app. The typical message looks like this:

severity: Error
type: Validation
message ID name: SYNC-HAZARD-WRITE-AFTER-WRITE
message ID: 0x5C0EC5D6
queues: N/A
command buffers: N/A
objects: [nullptr]
message: Validation Error: [ SYNC-HAZARD-WRITE-AFTER-WRITE ] Object 0: handle = 0xb400007ed9519e68,
    type = VK_OBJECT_TYPE_QUEUE; | MessageID = 0x5c0ec5d6 | vkQueueSubmit():  Hazard WRITE_AFTER_WRITE for entry 0,
    VkCommandBuffer 0xb400007e37fad308[Frame in flight #1], Submitted access info (submitted_usage:
    SYNC_COMPUTE_SHADER_SHADER_STORAGE_WRITE, command: vkCmdDispatch, seq_no: 277, reset_no: 2, debug_region: Exposure).
    Access info (prior_usage: SYNC_COMPUTE_SHADER_SHADER_STORAGE_WRITE, write_barriers: 0, queue: 
    VkQueue 0xb400007ed9519e68[], submit: 209, batch: 0, batch_tag: 2152, command: vkCmdDispatch,
    command_buffer: VkCommandBuffer 0xb400007e776cec68[Frame in flight #0], seq_no: 284, reset_no: 2,
    debug_region: Exposure).

Expected behavior

Validation warning contains resource type and name.

Valid Usage ID N/A

artem-lunarg commented 3 months ago

@Goshido there is a limitation to the amount of information that can be provided for the prior_usage access, but it should be possible to provide resource information for the submitted_usage.

Goshido commented 3 months ago

Quick investigation: It turns out that 3 VkBuffer do not have proper sync. Here is compute shader snippet for them:

[[vk::binding ( BIND_SYNC_MIP_5, SET_RESOURCE )]]
globallycoherent RWTexture2D<float32_t>             g_syncMip5:         register ( u0 );

[[vk::binding ( BIND_EXPOSURE, SET_RESOURCE )]]
RWStructuredBuffer<float32_t>                       g_exposure:         register ( u1 );

[[vk::binding ( BIND_GLOBAL_ATOMIC, SET_RESOURCE )]]
globallycoherent RWStructuredBuffer<uint32_t>       g_globalAtomic:     register ( u2 );

[[vk::binding ( BIND_TEMPORAL_LUMA, SET_RESOURCE )]]
RWStructuredBuffer<float32_t>                       g_temporalLuma:     register ( u3 );
Goshido commented 3 months ago

@artem-lunarg could I expect an improvement or it's super strong limitation for VVL architecture?

artem-lunarg commented 3 months ago

I'll try to add resource information to submitted_usage, does not look hard except I missed something. Adding more information to prior_usage is much harder, this requires to store a lot of information. We plan improvements, but there might be a tendency that current access information is more detailed compared to the previous access.

Goshido commented 3 months ago

Great! I gonna wait. Thx 👍

artem-lunarg commented 3 months ago

@Goshido Going to postpone the fix. Doable, but need to update memory allocation scheme first.

Implementation note: instead of allocating handle array per usage record, allocate single buffer per command buffer context and store only index/count in the usage record. This will avoid performance impact from large number of allocations. Memory wise single buffer solution is easy to manage (e.g. disable storing handles when memory is a concern) and easy to collect stats.

artem-lunarg commented 3 months ago

@Goshido Support is added here https://github.com/KhronosGroup/Vulkan-ValidationLayers/pull/8222. Currently it's buffers only, images will be later after we test current solution for a bit.

Goshido commented 2 months ago

Awesome 👍