Skip to content

Commit

Permalink
layers: Fix checks with runtime arrays
Browse files Browse the repository at this point in the history
  • Loading branch information
spencer-lunarg committed Jun 19, 2024
1 parent 346dc33 commit a28c326
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 8 deletions.
5 changes: 4 additions & 1 deletion layers/drawdispatch/descriptor_validator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,10 @@ bool vvl::DescriptorValidator::ValidateDescriptor(const DescriptorBindingInfo &b
string_SpvDim(dim), is_image_array);
}

if ((variable->info.image_format_type & image_view_state->descriptor_format_bits) == 0) {
// Because you can have a runtime array with different types in it, without extensive GPU-AV tracking, we have no way to
// detect if the types match up in a given index
if (variable->array_length != spirv::kRuntimeArray &&
((variable->info.image_format_type & image_view_state->descriptor_format_bits) == 0)) {
const bool signed_override =
((variable->info.image_format_type & spirv::NumericTypeUint) && variable->info.is_sign_extended);
const bool unsigned_override =
Expand Down
8 changes: 6 additions & 2 deletions layers/state_tracker/shader_module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1856,6 +1856,8 @@ const Instruction& ResourceInterfaceVariable::FindBaseType(ResourceInterfaceVari
// currently just tracks 1D arrays
if (type->Opcode() == spv::OpTypeArray && variable.array_length == 0) {
variable.array_length = module_state.GetConstantValueById(type->Word(3));
} else if (type->Opcode() == spv::OpTypeRuntimeArray) {
variable.array_length = spirv::kRuntimeArray;
}

if (type->Opcode() == spv::OpTypeSampledImage) {
Expand Down Expand Up @@ -1964,7 +1966,9 @@ ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state,
if (is_sampled_without_sampler) {
if (info.image_dim == spv::DimSubpassData) {
is_input_attachment = true;
input_attachment_index_read.resize(array_length); // is zero if runtime array
if (array_length != spirv::kRuntimeArray) {
input_attachment_index_read.resize(array_length);
}
} else if (info.image_dim == spv::DimBuffer) {
is_storage_texel_buffer = true;
} else {
Expand All @@ -1987,7 +1991,7 @@ ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state,
info.is_zero_extended |= image_access.is_zero_extended;
access_mask |= image_access.access_mask;

if (array_length > 1) {
if (array_length > 1 && array_length != spirv::kRuntimeArray) {
image_access_chain_indexes.insert(image_access.image_access_chain_index);
}

Expand Down
4 changes: 4 additions & 0 deletions layers/state_tracker/shader_module.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ struct Module;

static constexpr uint32_t kInvalidValue = std::numeric_limits<uint32_t>::max();

// Need to find a way to know if actually array lenght of zero, or a runtime array.
static constexpr uint32_t kRuntimeArray = std::numeric_limits<uint32_t>::max();

// This is the common info for both OpDecorate and OpMemberDecorate
// Used to keep track of all decorations applied to any instruction
struct DecorationBase {
Expand Down Expand Up @@ -371,6 +374,7 @@ struct StageInterfaceVariable : public VariableBase {
// at draw/submit time we can cross reference with the last bound descriptor.
struct ResourceInterfaceVariable : public VariableBase {
// If the type is a OpTypeArray save the length
// Will be kRuntimeArray (non-zero) for runtime arrays
uint32_t array_length;

bool is_sampled_image; // OpTypeSampledImage
Expand Down
76 changes: 71 additions & 5 deletions tests/unit/gpu_av_positive.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,72 @@ TEST_F(PositiveGpuAV, AliasImageBinding) {
vk::DeviceWaitIdle(*m_device);
}

TEST_F(PositiveGpuAV, AliasImageBindingNonFixed) {
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/7677");
AddRequiredExtensions(VK_EXT_DESCRIPTOR_INDEXING_EXTENSION_NAME);
AddRequiredFeature(vkt::Feature::runtimeDescriptorArray);
AddRequiredFeature(vkt::Feature::shaderSampledImageArrayNonUniformIndexing);
RETURN_IF_SKIP(InitGpuAvFramework());
RETURN_IF_SKIP(InitState());

char const *csSource = R"glsl(
#version 460
#extension GL_EXT_nonuniform_qualifier : enable
#extension GL_EXT_samplerless_texture_functions : require
layout(set = 0, binding = 0) uniform texture2D float_textures[];
layout(set = 0, binding = 0) uniform utexture2D uint_textures[];
layout(set = 0, binding = 1) buffer output_buffer {
uint index;
vec4 data;
};
void main() {
const vec4 value = texelFetch(float_textures[nonuniformEXT(index)], ivec2(0), 0);
const uint mask = texelFetch(uint_textures[nonuniformEXT(index + 1)], ivec2(0), 0).x;
data = mask > 0 ? value : vec4(0.0);
}
)glsl";

CreateComputePipelineHelper pipe(*this);
pipe.dsl_bindings_ = {{0, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE, 2, VK_SHADER_STAGE_ALL, nullptr},
{1, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER, 1, VK_SHADER_STAGE_ALL, nullptr}};
pipe.cs_ = std::make_unique<VkShaderObj>(this, csSource, VK_SHADER_STAGE_COMPUTE_BIT);
pipe.CreateComputePipeline();

auto image_ci = vkt::Image::ImageCreateInfo2D(64, 64, 1, 1, VK_FORMAT_R8G8B8A8_UNORM, VK_IMAGE_USAGE_SAMPLED_BIT);
vkt::Image float_image(*m_device, image_ci);
float_image.SetLayout(VK_IMAGE_LAYOUT_GENERAL);
vkt::ImageView float_image_view = float_image.CreateView();

image_ci.format = VK_FORMAT_R8G8B8A8_UINT;
vkt::Image uint_image(*m_device, image_ci);
uint_image.SetLayout(VK_IMAGE_LAYOUT_GENERAL);
vkt::ImageView uint_image_view = uint_image.CreateView();

vkt::Buffer buffer(*m_device, 64, VK_BUFFER_USAGE_STORAGE_BUFFER_BIT,
VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | VK_MEMORY_PROPERTY_HOST_COHERENT_BIT);
uint32_t *data = (uint32_t *)buffer.memory().map();
*data = 0;
buffer.memory().unmap();

pipe.descriptor_set_->WriteDescriptorImageInfo(0, float_image_view.handle(), VK_NULL_HANDLE, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE,
VK_IMAGE_LAYOUT_GENERAL, 0);
pipe.descriptor_set_->WriteDescriptorImageInfo(0, uint_image_view.handle(), VK_NULL_HANDLE, VK_DESCRIPTOR_TYPE_SAMPLED_IMAGE,
VK_IMAGE_LAYOUT_GENERAL, 1);
pipe.descriptor_set_->WriteDescriptorBufferInfo(1, buffer.handle(), 0, VK_WHOLE_SIZE, VK_DESCRIPTOR_TYPE_STORAGE_BUFFER);
pipe.descriptor_set_->UpdateDescriptorSets();

m_commandBuffer->begin();
vk::CmdBindDescriptorSets(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.pipeline_layout_.handle(), 0, 1,
&pipe.descriptor_set_->set_, 0, nullptr);
vk::CmdBindPipeline(m_commandBuffer->handle(), VK_PIPELINE_BIND_POINT_COMPUTE, pipe.Handle());
vk::CmdDispatch(m_commandBuffer->handle(), 1, 1, 1);
m_commandBuffer->end();
m_default_queue->Submit(*m_commandBuffer);
vk::DeviceWaitIdle(*m_device);
}

TEST_F(PositiveGpuAV, SwapchainImage) {
TEST_DESCRIPTION("https://github.com/KhronosGroup/Vulkan-ValidationLayers/issues/8091");
AddSurfaceExtension();
Expand Down Expand Up @@ -1331,13 +1397,13 @@ TEST_F(PositiveGpuAV, RestoreUserPushConstants) {
} pc;
vec2 vertices[3];
void main() {
vertices[0] = vec2(-1.0, -1.0);
vertices[1] = vec2( 1.0, -1.0);
vertices[2] = vec2( 0.0, 1.0);
gl_Position = vec4(vertices[gl_VertexIndex % 3], 0.0, 1.0);
for (int i = 0; i < 8; ++i) {
pc.ptr.out_array[i] = pc.in_array[i];
}
Expand Down Expand Up @@ -1436,13 +1502,13 @@ TEST_F(PositiveGpuAV, RestoreUserPushConstants2) {
} pc;
vec2 vertices[3];
void main() {
vertices[0] = vec2(-1.0, -1.0);
vertices[1] = vec2( 1.0, -1.0);
vertices[2] = vec2( 0.0, 1.0);
gl_Position = vec4(vertices[gl_VertexIndex % 3], 0.0, 1.0);
for (int i = 0; i < 4; ++i) {
pc.ptr.out_array[i] = pc.in_array[i];
}
Expand Down Expand Up @@ -1523,7 +1589,7 @@ TEST_F(PositiveGpuAV, RestoreUserPushConstants2) {
} pc;
layout (local_size_x = 256, local_size_y = 1, local_size_z = 1) in;
void main() {
for (int i = 0; i < 8; ++i) {
pc.ptr.out_array[i] = pc.in_array[i];
Expand Down

0 comments on commit a28c326

Please sign in to comment.