Skip to content

Commit

Permalink
layers: Mark OpArrayLength as static usage
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Jan 23, 2025
1 parent 0b87333 commit cfe6a55
Show file tree
Hide file tree
Showing 6 changed files with 128 additions and 9 deletions.
2 changes: 1 addition & 1 deletion layers/state_tracker/pipeline_state.h
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ class Pipeline : public StateObject {

const vvl::unordered_set<uint32_t> fragmentShader_writable_output_location_list;

// NOTE: this map is 'almost' const and used in performance critical code paths.
// NOTE: this map is used in performance critical code paths.
// The values of existing entries in the samplers_used_by_image map
// are updated at various times. Locking requirements are TBD.
const ActiveSlotMap active_slots;
Expand Down
6 changes: 6 additions & 0 deletions layers/state_tracker/shader_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@

#include "utils/hash_util.h"
#include "generated/spirv_grammar_helper.h"

#include <spirv/unified1/spirv.hpp>
#include <spirv/1.2/GLSL.std.450.h>
#include <spirv/unified1/NonSemanticShaderDebugInfo100.h>
#include "error_message/spirv_logging.h"
Expand Down Expand Up @@ -347,6 +349,10 @@ static void FindPointersAndObjects(const Instruction& insn, vvl::unordered_set<u
case spv::OpInBoundsAccessChain:
result.insert(insn.Word(3)); // base ptr
break;
case spv::OpArrayLength:
// This is not an access of memory, but counts as static usage of the variable
result.insert(insn.Word(3));
break;
case spv::OpSampledImage:
case spv::OpImageSampleImplicitLod:
case spv::OpImageSampleExplicitLod:
Expand Down
10 changes: 6 additions & 4 deletions layers/state_tracker/shader_stage_state.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Copyright (c) 2024 The Khronos Group Inc.
* Copyright (c) 2024 Valve Corporation
* Copyright (c) 2024 LunarG, Inc.
/* Copyright (c) 2024-2025 The Khronos Group Inc.
* Copyright (c) 2024-2025 Valve Corporation
* Copyright (c) 2024-2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -21,6 +21,7 @@
#include "state_tracker/shader_module.h"
#include <vulkan/utility/vk_safe_struct.hpp>

// Common for both Pipeline and Shader Object
void GetActiveSlots(ActiveSlotMap &active_slots, const std::shared_ptr<const spirv::EntryPoint> &entrypoint) {
if (!entrypoint) {
return;
Expand All @@ -35,7 +36,7 @@ void GetActiveSlots(ActiveSlotMap &active_slots, const std::shared_ptr<const spi
}
}

// static
// Used by pipeline
ActiveSlotMap GetActiveSlots(const std::vector<ShaderStageState> &stage_states) {
ActiveSlotMap active_slots;
for (const auto &stage : stage_states) {
Expand All @@ -44,6 +45,7 @@ ActiveSlotMap GetActiveSlots(const std::vector<ShaderStageState> &stage_states)
return active_slots;
}

// Used by Shader Object
ActiveSlotMap GetActiveSlots(const std::shared_ptr<const spirv::EntryPoint> &entrypoint) {
ActiveSlotMap active_slots;
GetActiveSlots(active_slots, entrypoint);
Expand Down
9 changes: 5 additions & 4 deletions layers/state_tracker/shader_stage_state.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* Copyright (c) 2024 The Khronos Group Inc.
* Copyright (c) 2024 Valve Corporation
* Copyright (c) 2024 LunarG, Inc.
/* Copyright (c) 2024-2025 The Khronos Group Inc.
* Copyright (c) 2024-2025 Valve Corporation
* Copyright (c) 2024-2025 LunarG, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -79,7 +79,8 @@ inline bool operator<(const DescriptorRequirement &a, const DescriptorRequiremen
// < binding index (of descriptor set) : meta data >
typedef std::unordered_multimap<uint32_t, DescriptorRequirement> BindingVariableMap;

// Capture which slots (set#->bindings) are actually used by the shaders of this pipeline
// Capture which slots (set#->bindings) are actually used by the shaders of this pipeline/shaderObject.
// This is same as "statically used" in vkspec.html#shaders-staticuse
using ActiveSlotMap = vvl::unordered_map<uint32_t, BindingVariableMap>;

void GetActiveSlots(ActiveSlotMap &active_slots, const std::shared_ptr<const spirv::EntryPoint> &entrypoint);
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/descriptors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,65 @@ TEST_F(NegativeDescriptors, CmdBufferDescriptorSetImageSamplerDestroyed) {
m_errorMonitor->VerifyFound();
}

TEST_F(NegativeDescriptors, OpArrayLengthStaticallyUsed) {
TEST_DESCRIPTION("https://gitlab.khronos.org/vulkan/vulkan/-/merge_requests/7137");
SetTargetApiVersion(VK_API_VERSION_1_2);
RETURN_IF_SKIP(Init());
m_errorMonitor->ExpectSuccess(kErrorBit | kWarningBit | kInformationBit);

char const *shader_source = R"glsl(
#version 450
#extension GL_EXT_debug_printf : enable
layout(set = 0, binding = 0) buffer SSBO_0 {
uint a;
};
layout(set = 1, binding = 0) buffer SSBO_1 {
uint b;
vec4 c[];
};
void main() {
// length() here is consider static usage of the descriptor
a = c.length();
}
)glsl";

vkt::Buffer buffer(*m_device, 256, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT);
OneOffDescriptorSet descriptor_set0(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
OneOffDescriptorSet descriptor_set1(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set0.layout_, &descriptor_set1.layout_});
descriptor_set0.WriteDescriptorBufferInfo(0, buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
descriptor_set0.UpdateDescriptorSets();
descriptor_set1.WriteDescriptorBufferInfo(0, buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
descriptor_set1.UpdateDescriptorSets();

CreateComputePipelineHelper pipe(*this);
pipe.cp_ci_.layout = pipeline_layout.handle();
pipe.cs_ = std::make_unique<VkShaderObj>(this, shader_source, VK_SHADER_STAGE_COMPUTE_BIT);
pipe.CreateComputePipeline();

m_command_buffer.Begin();
vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.Handle());
vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 0, 1,
&descriptor_set0.set_, 0, nullptr);
vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 1, 1,
&descriptor_set1.set_, 0, nullptr);
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
m_command_buffer.End();

m_default_queue->Submit(m_command_buffer);
m_default_queue->Wait();

descriptor_set1.UpdateDescriptorSets(); // command buffer is invalid now

m_errorMonitor->SetDesiredError("VUID-vkQueueSubmit-pCommandBuffers-00070");
m_default_queue->Submit(m_command_buffer);
m_default_queue->Wait();
m_errorMonitor->VerifyFound();
}

TEST_F(NegativeDescriptors, DescriptorSetSamplerDestroyed) {
TEST_DESCRIPTION("Attempt to draw with a bound descriptor sets with a combined image sampler where sampler has been deleted.");
RETURN_IF_SKIP(Init());
Expand Down
51 changes: 51 additions & 0 deletions tests/unit/gpu_av_descriptor_class_general_buffer_positive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1444,3 +1444,54 @@ TEST_F(PositiveGpuAVDescriptorClassGeneralBuffer, DescriptorIndexSlang) {
m_default_queue->Submit(m_command_buffer);
m_default_queue->Wait();
}

TEST_F(PositiveGpuAVDescriptorClassGeneralBuffer, OpArrayLength) {
TEST_DESCRIPTION("https://gitlab.khronos.org/vulkan/vulkan/-/issues/4145");
RETURN_IF_SKIP(InitGpuAvFramework());
RETURN_IF_SKIP(InitState());

char const *cs_source = R"glsl(
#version 450
#extension GL_EXT_debug_printf : enable
layout(set = 0, binding = 0) buffer SSBO_0 {
uint a;
};
layout(set = 1, binding = 0) buffer SSBO_1 {
vec4 b;
uint c[];
};
void main() {
// length() here is NOT an access
a = c.length();
}
)glsl";

vkt::Buffer buffer(*m_device, 32, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT);
OneOffDescriptorSet descriptor_set0(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
OneOffDescriptorSet descriptor_set1(m_device, {{0, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}});
const vkt::PipelineLayout pipeline_layout(*m_device, {&descriptor_set0.layout_, &descriptor_set1.layout_});
descriptor_set0.WriteDescriptorBufferInfo(0, buffer, 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
descriptor_set0.UpdateDescriptorSets();
descriptor_set1.WriteDescriptorBufferInfo(0, buffer, 0, 16, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
descriptor_set1.UpdateDescriptorSets();

CreateComputePipelineHelper pipe(*this);
pipe.cp_ci_.layout = pipeline_layout.handle();
pipe.cs_ = std::make_unique<VkShaderObj>(this, cs_source, VK_SHADER_STAGE_COMPUTE_BIT, SPV_ENV_VULKAN_1_1);
pipe.CreateComputePipeline();

m_command_buffer.Begin();
vk::CmdBindPipeline(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.Handle());
vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 0, 1,
&descriptor_set0.set_, 0, nullptr);
vk::CmdBindDescriptorSets(m_command_buffer.handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipeline_layout.handle(), 1, 1,
&descriptor_set1.set_, 0, nullptr);
vk::CmdDispatch(m_command_buffer.handle(), 1, 1, 1);
m_command_buffer.End();

m_default_queue->Submit(m_command_buffer);
m_default_queue->Wait();
}

0 comments on commit cfe6a55

Please sign in to comment.