gpu: More tests and docs around Post Processing

docs/gpu_av_post_process.md

@@ -36,33 +36,10 @@ The goal is that the `ValidateDescriptor()` function can be called the same from
 
 When tracking data for Post Process, we need both information from `vkCmdBindDescriptorSets` and `vkCmdBindPipeline` (or shader objects), but not all are present at the same time.
 
-At action command time we need to bind a `VkBuffer` that contains the Buffer Device Address to each descriptor set. If we have 4 descriptor set and the following command buffer stream:
+The `VkDescriptorSet` object contains a buffer with a `PostProcessDescriptorIndexSlot` for each descriptor in it. This "descriptor index slot" holds information that can only be known at GPU execution time (which index, OpVariable, etc... are used). For each action command, we create a LUT mapping 32 (current max limit) addresses to the `VkDescriptorSet` buffer.
 
-```c++
-vkCmdBindDescriptorSets(firstSet = 0, setCount = 2, [set_a, set_b]);
-vkCmdDraw()
-vkCmdBindDescriptorSets(firstSet = 3, setCount = 1, [set_a]);
-vkCmdDraw()
-```
-
-the first draw the `VkBuffer` contents would look like
-
-```
-[&set_a, &set_b, null, null]
-   |        |
-   |      [descriptor_0, descriptor_1, etc]
-   |
- [descriptor_0, descriptor_1, etc]
-```
-
-and the second draw would look like
+The pipelines contain the `OpVariable` SPIR-V information, such as which type of image is used. This is wrapped up in a `BindingVariableMap` pointer. Using this we can map the accessed descriptor on the GPU back to all the state tracking on the CPU.
 
-```
-[&set_a, &set_b, null, &set_a]
-   |        |            |
-   |        -------------|------[descriptor_0, descriptor_1, etc]
-   |                     |
- [descriptor_0, descriptor_1, etc]
-```
+The following diagram aims to illustrate the flow of data:
 
-With this information, we now need to pair that up with the SPIR-V information. This information is saved in a `BindingVariableMap` pointer. This contains a mapping between each binding in the descriptor set, and what each `OpVariable` tied to it looks like.
+![alt_text](images/post_processing.svg "Post Processing")

layers/gpu/descriptor_validation/gpuav_descriptor_validation.cpp

@@ -91,7 +91,7 @@ void UpdateBoundDescriptorsPostProcess(Validator &gpuav, CommandBuffer &cb_state
     auto ssbo_buffer_ptr = (glsl::PostProcessSSBO *)descriptor_command_binding.post_process_ssbo_buffer.MapMemory(loc);
     memset(ssbo_buffer_ptr, 0, sizeof(glsl::PostProcessSSBO));
 
-    cb_state.post_process_buffer = descriptor_command_binding.post_process_ssbo_buffer.VkHandle();
+    cb_state.post_process_buffer_lut = descriptor_command_binding.post_process_ssbo_buffer.VkHandle();
 
     const size_t number_of_sets = last_bound.ds_slots.size();
     for (uint32_t i = 0; i < number_of_sets; i++) {
@@ -262,7 +262,9 @@ void UpdateBoundDescriptors(Validator &gpuav, CommandBuffer &cb_state, VkPipelin
                         break;
                     }
                 }
-                ASSERT_AND_CONTINUE(resource_variable);
+
+                // This can occur if 2 shaders have different OpVariable, but the pipelines are sharing the same descriptor set
+                if (!resource_variable) continue;
 
                 // If we already validated/updated the descriptor on the CPU, don't redo it now in GPU-AV Post Processing
                 if (!bound_descriptor_set->ValidateBindingOnGPU(*descriptor_binding,

layers/gpu/descriptor_validation/gpuav_image_layout.cpp

@@ -203,13 +203,13 @@ static bool VerifyImageLayoutRange(const Validator &gpuav, const vvl::CommandBuf
                     const LogObjectList objlist(cb_state.Handle(), image_state.Handle());
                     // TODO - We need a way to map the action command to which caused this error
                     const vvl::DrawDispatchVuid &vuid = GetDrawDispatchVuid(vvl::Func::vkCmdDraw);
-                    skip |= gpuav.LogError(vuid.image_layout_09600, objlist, loc,
-                                           "command buffer %s expects %s (subresource: aspectMask %s, array layer %" PRIu32
-                                           ", mip level %" PRIu32 ") to be in layout %s--instead, current layout is %s.",
-                                           gpuav.FormatHandle(cb_state).c_str(), gpuav.FormatHandle(image_state).c_str(),
-                                           string_VkImageAspectFlags(subresource.aspectMask).c_str(), subresource.arrayLayer,
-                                           subresource.mipLevel, string_VkImageLayout(initial_layout),
-                                           string_VkImageLayout(image_layout));
+                    skip |= gpuav.LogError(
+                        vuid.image_layout_09600, objlist, loc,
+                        "command buffer %s expects %s (subresource: aspectMask %s, array layer %" PRIu32 ", mip level %" PRIu32
+                        ") to be in layout %s--instead, current layout is %s. (Detected from GPU-AV)",
+                        gpuav.FormatHandle(cb_state).c_str(), gpuav.FormatHandle(image_state).c_str(),
+                        string_VkImageAspectFlags(subresource.aspectMask).c_str(), subresource.arrayLayer, subresource.mipLevel,
+                        string_VkImageLayout(initial_layout), string_VkImageLayout(image_layout));
                 }
             }
         }

layers/gpu/instrumentation/gpuav_instrumentation.cpp

@@ -213,9 +213,9 @@ void UpdateInstrumentationDescSet(Validator &gpuav, CommandBuffer &cb_state, VkD
 
     // Post Processing Output buffer
     VkDescriptorBufferInfo post_process_buffer_info = {};
-    if (cb_state.post_process_buffer != VK_NULL_HANDLE) {
+    if (cb_state.post_process_buffer_lut != VK_NULL_HANDLE) {
         post_process_buffer_info.range = VK_WHOLE_SIZE;
-        post_process_buffer_info.buffer = cb_state.post_process_buffer;
+        post_process_buffer_info.buffer = cb_state.post_process_buffer_lut;
         post_process_buffer_info.offset = 0;
 
         VkWriteDescriptorSet wds = vku::InitStructHelper();

layers/gpu/resources/gpuav_state_trackers.cpp

@@ -393,7 +393,7 @@ void CommandBuffer::ResetCBState() {
     descriptor_command_bindings.clear();
     action_command_snapshots.clear();
     descriptor_indexing_buffer = VK_NULL_HANDLE;
-    post_process_buffer = VK_NULL_HANDLE;
+    post_process_buffer_lut = VK_NULL_HANDLE;
 
     error_output_buffer_.Destroy();
     cmd_errors_counts_buffer_.Destroy();

layers/gpu/resources/gpuav_state_trackers.h

@@ -64,7 +64,7 @@ class CommandBuffer : public vvl::CommandBuffer {
 
     // Buffer to be bound every draw/dispatch/action
     VkBuffer descriptor_indexing_buffer = VK_NULL_HANDLE;
-    VkBuffer post_process_buffer = VK_NULL_HANDLE;
+    VkBuffer post_process_buffer_lut = VK_NULL_HANDLE;
 
     // Used to track which spot in the command buffer the error came from
     uint32_t draw_index = 0;

tests/unit/gpu_av_descriptor_indexing.cpp

@@ -3639,3 +3639,205 @@ TEST_F(NegativeGpuAVDescriptorIndexing, StorageImageRuntimeArray) {
     m_default_queue->Wait();
     m_errorMonitor->VerifyFound();
 }
+
+TEST_F(NegativeGpuAVDescriptorIndexing, MultipleCommandBuffersSameDescriptorSet) {
+    TEST_DESCRIPTION("two command buffers share same descriptor set, the second one makes invalid access");
+    RETURN_IF_SKIP(InitGpuVUDescriptorIndexing());
+
+    vkt::CommandBuffer cb_0(*m_device, m_command_pool);
+    vkt::CommandBuffer cb_1(*m_device, m_command_pool);
+
+    OneOffDescriptorIndexingSet descriptor_set(m_device,
+                                               {
+                                                   {0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_ALL, nullptr,
+                                                    VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT},
+                                                   {1, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr, 0},
+                                               });
+    const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set.layout_});
+
+    vkt::Buffer buffer(*m_device, 64, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, kHostVisibleMemProps);
+    vkt::Sampler sampler(*m_device, SafeSaneSamplerCreateInfo());
+    auto image_ci = vkt::Image::ImageCreateInfo2D(16, 16, 1, 1, VK_FORMAT_B8G8R8A8_UNORM, VK_IMAGE_USAGE_SAMPLED_BIT);
+    vkt::Image image(*m_device, image_ci, vkt::set_layout);
+    vkt::ImageView image_view = image.CreateView();
+
+    descriptor_set.WriteDescriptorImageInfo(0, image_view, sampler, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
+                                            VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, 0);
+    descriptor_set.WriteDescriptorBufferInfo(1, buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
+    descriptor_set.UpdateDescriptorSets();
+
+    char const *cs_source0 = R"glsl(
+        #version 450
+        #extension GL_EXT_nonuniform_qualifier : enable
+        layout(set=0, binding=0) uniform sampler2D sample_array[]; // only [0] is set
+        layout(set=0, binding=1) buffer SSBO {
+            uint index;
+            vec4 out_value;
+        };
+        void main() {
+           out_value = texture(sample_array[index], vec2(0));
+        }
+    )glsl";
+    CreateComputePipelineHelper pipe0(*this);
+    pipe0.cs_ = std::make_unique<VkShaderObj>(this, cs_source0, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_2);
+    pipe0.cp_ci_.layout = pipeline_layout.handle();
+    pipe0.CreateComputePipeline();
+
+    // Create different shaders so pipelines are different
+    char const *cs_source1 = R"glsl(
+        #version 450
+        #extension GL_EXT_nonuniform_qualifier : enable
+        layout(set=0, binding=0) uniform sampler2D sample_array[2];
+        layout(set=0, binding=1) buffer SSBO {
+            uint index;
+            float some_padding;
+            vec4 out_value;
+        };
+        void main() {
+           out_value = texture(sample_array[index], vec2(0));
+        }
+    )glsl";
+    CreateComputePipelineHelper pipe1(*this);
+    pipe1.cs_ = std::make_unique<VkShaderObj>(this, cs_source1, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_2);
+    pipe1.cp_ci_.layout = pipeline_layout.handle();
+    pipe1.CreateComputePipeline();
+
+    cb_0.Begin();
+    vk::CmdBindPipeline(cb_0.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe0.Handle());
+    vk::CmdBindDescriptorSets(cb_0.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1, &descriptor_set.set_, 0,
+                              nullptr);
+    vk::CmdDispatch(cb_0.handle(), 1, 1, 1);
+    cb_0.End();
+
+    cb_1.Begin();
+    vk::CmdBindPipeline(cb_1.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe1.Handle());
+    vk::CmdBindDescriptorSets(cb_1.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1, &descriptor_set.set_, 0,
+                              nullptr);
+    vk::CmdDispatch(cb_1.handle(), 1, 1, 1);
+    cb_1.End();
+
+    uint32_t *buffer_ptr = (uint32_t *)buffer.Memory().Map();
+    buffer_ptr[0] = 0;
+
+    m_default_queue->Submit(cb_0);
+    m_default_queue->Wait();
+
+    buffer_ptr[0] = 1;
+    m_errorMonitor->SetDesiredError("VUID-vkCmdDispatch-None-08114");
+    m_default_queue->Submit(cb_1);
+    m_default_queue->Wait();
+    m_errorMonitor->VerifyFound();
+
+    // Invalid access should not carry back over
+    buffer_ptr[0] = 0;
+    m_default_queue->Submit(cb_0);
+    m_default_queue->Wait();
+
+    buffer.Memory().Unmap();
+}
+
+TEST_F(NegativeGpuAVDescriptorIndexing, CommandBufferRerecordSameDescriptorSet) {
+    TEST_DESCRIPTION("rerecord the command buffer with an invalid pipeline the second time");
+    RETURN_IF_SKIP(InitGpuVUDescriptorIndexing());
+
+    OneOffDescriptorIndexingSet descriptor_set(m_device,
+                                               {
+                                                   {0, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER, 2, VK_SHADER_STAGE_ALL, nullptr,
+                                                    VK_DESCRIPTOR_BINDING_PARTIALLY_BOUND_BIT},
+                                                   {1, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr, 0},
+                                               });
+    const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set.layout_});
+
+    vkt::Buffer buffer(*m_device, 64, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, kHostVisibleMemProps);
+    uint32_t *buffer_ptr = (uint32_t *)buffer.Memory().Map();
+    buffer_ptr[0] = 0;
+    buffer_ptr[1] = 1;
+    buffer.Memory().Unmap();
+
+    vkt::Sampler sampler(*m_device, SafeSaneSamplerCreateInfo());
+    auto image_ci = vkt::Image::ImageCreateInfo2D(16, 16, 1, 1, VK_FORMAT_B8G8R8A8_UNORM, VK_IMAGE_USAGE_SAMPLED_BIT);
+    vkt::Image image(*m_device, image_ci, vkt::set_layout);
+    vkt::ImageView image_view = image.CreateView();
+
+    descriptor_set.WriteDescriptorImageInfo(0, image_view, sampler, VK_DESCRIPTOR_TYPE_COMBINED_IMAGE_SAMPLER,
+                                            VK_IMAGE_LAYOUT_SHADER_READ_ONLY_OPTIMAL, 0);
+    descriptor_set.WriteDescriptorBufferInfo(1, buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
+    descriptor_set.UpdateDescriptorSets();
+
+    char const *cs_source0 = R"glsl(
+        #version 450
+        #extension GL_EXT_nonuniform_qualifier : enable
+        layout(set=0, binding=0) uniform sampler2D sample_array[]; // only [0] is set
+        layout(set=0, binding=1) buffer SSBO {
+            uint index_0; // has value 0
+            uint index_1; // has value 1
+            vec4 out_value;
+        };
+        void main() {
+           out_value = texture(sample_array[index_0], vec2(0));
+        }
+    )glsl";
+    CreateComputePipelineHelper pipe_good(*this);
+    pipe_good.cs_ = std::make_unique<VkShaderObj>(this, cs_source0, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_2);
+    pipe_good.cp_ci_.layout = pipeline_layout.handle();
+    pipe_good.CreateComputePipeline();
+
+    // Create different shaders so pipelines are different
+    char const *cs_source1 = R"glsl(
+        #version 450
+        #extension GL_EXT_nonuniform_qualifier : enable
+        layout(set=0, binding=0) uniform sampler2D sample_array[2];
+        layout(set=0, binding=1) buffer SSBO {
+            uint index_0; // has value 0
+            uint index_1; // has value 1
+            float some_padding;
+            vec4 out_value;
+        };
+        void main() {
+           out_value = texture(sample_array[index_1], vec2(0));
+        }
+    )glsl";
+    CreateComputePipelineHelper pipe_bad(*this);
+    pipe_bad.cs_ = std::make_unique<VkShaderObj>(this, cs_source1, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_2);
+    pipe_bad.cp_ci_.layout = pipeline_layout.handle();
+    pipe_bad.CreateComputePipeline();
+
+    m_command_buffer.Begin();
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_bad.Handle());
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_good.Handle());
+    vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1,
+                              &descriptor_set.set_, 0, nullptr);
+    vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
+
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_bad.Handle());
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_good.Handle());
+    vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
+    m_command_buffer.End();
+
+    m_default_queue->Submit(m_command_buffer);
+    m_default_queue->Wait();
+
+    // record with bad pipeline
+    m_command_buffer.Begin();
+    vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1,
+                              &descriptor_set.set_, 0, nullptr);
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_bad.Handle());
+    vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
+    m_command_buffer.End();
+
+    m_errorMonitor->SetDesiredError("VUID-vkCmdDispatch-None-08114");
+    m_default_queue->Submit(m_command_buffer);
+    m_default_queue->Wait();
+    m_errorMonitor->VerifyFound();
+
+    // Re-record to make sure VU doesn't presist
+    m_command_buffer.Begin();
+    vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout, 0, 1,
+                              &descriptor_set.set_, 0, nullptr);
+    vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe_good.Handle());
+    vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
+    m_command_buffer.End();
+
+    m_default_queue->Submit(m_command_buffer);
+    m_default_queue->Wait();
+}