Skip to content

Commit d12550f

Browse files
spencer-lunargspencer-lunarg
committed
layers: Combine ResourceInterfaceVariable info
1 parent bf296c8 commit d12550f

8 files changed

Lines changed: 110 additions & 110 deletions

File tree

‎layers/core_checks/cc_spirv.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,7 +1801,7 @@ bool CoreChecks::ValidateShaderYcbcrSampler(const spirv::Module& module_state, c
18011801
}
18021802

18031803
// YCbCr is only allowed for Combined Image Samplers (error is caught before)
1804-
if (!variable.is_type_sampled_image) {
1804+
if (!variable.is_combined_image_sampler) {
18051805
return skip;
18061806
}
18071807

@@ -3365,9 +3365,9 @@ bool CoreChecks::ValidateShaderDescriptorSetAndBindingMappingInfo(const spirv::M
33653365
VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_EXT,
33663366
VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_INDIRECT_INDEX_ARRAY_EXT,
33673367
VK_DESCRIPTOR_MAPPING_SOURCE_HEAP_WITH_SHADER_RECORD_INDEX_EXT})) {
3368-
const spirv::Instruction& base_type = resource_variable.base_type;
3369-
const uint32_t base_opcode = base_type.Opcode();
3370-
const bool is_sampler = (base_opcode == spv::OpTypeSampledImage) || resource_variable.is_type_sampled_image;
3368+
const uint32_t base_opcode = resource_variable.base_type.Opcode();
3369+
// TODO - Seem we are missing the image portion of combined image samplers
3370+
const bool is_sampler = resource_variable.is_combined_image_sampler;
33713371

33723372
struct MappingSourceInfo {
33733373
uint32_t offset = 0;

‎layers/gpu_dump/gpu_dump_descriptor.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ void CommandBufferSubState::DumpDescriptorHeap(std::ostringstream& ss, const Las
309309
void Print(const CommandBufferSubState& cb_sub_state, std::ostringstream& map_ss, const VkPhysicalDevice physical_device) {
310310
map_ss << " - Binding " << std::dec << resource_variable->decorations.binding << ", "
311311
<< string_VkDescriptorMappingSourceEXT(mapping->source) << " (from pMappings[" << mapping_index << "])\n";
312-
const bool is_sampler =
313-
(resource_variable->base_type.Opcode() == spv::OpTypeSampledImage) || resource_variable->is_type_sampled_image;
312+
// TODO - should print both here
313+
const bool is_sampler = resource_variable->is_combined_image_sampler;
314314
const bool is_array = resource_variable->IsArray();
315315
uint32_t array_length = 0;
316316
if (is_array && !resource_variable->IsRuntimeArray() && resource_variable->array_length != spirv::kSpecConstant) {

‎layers/state_tracker/descriptor_sets.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -930,8 +930,11 @@ bool vvl::DescriptorSet::ValidateBindingOnGPU(const DescriptorBinding& binding,
930930
} else if (IsBindless(binding.binding_flags)) {
931931
// If flags allow descriptor to be "bindless" (can be invalid up until submit time)
932932
return true;
933-
} else if (variable.is_runtime_descriptor_array) {
933+
} else if (variable.array_length == spirv::kRuntimeArray) {
934934
// We don't know where OOB is on the CPU
935+
//
936+
// True if the Resource variable itself is runtime descriptor array
937+
// Online example to showcase various arrays we do/don't care about here https://godbolt.org/z/h9jhsKaPn
935938
return true;
936939
}
937940
return false;

‎layers/state_tracker/shader_instruction.cpp‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -205,15 +205,6 @@ bool Instruction::IsAccessChain() const {
205205
opcode == spv::OpInBoundsPtrAccessChain;
206206
}
207207

208-
spv::Dim Instruction::FindImageDim() const { return (Opcode() == spv::OpTypeImage) ? (spv::Dim(Word(3))) : spv::DimMax; }
209-
210-
bool Instruction::IsImageArray() const { return (Opcode() == spv::OpTypeImage) && (Word(5) != 0); }
211-
212-
bool Instruction::IsImageMultisampled() const {
213-
// spirv-val makes sure that the MS operand is only non-zero when possible to be Multisampled
214-
return (Opcode() == spv::OpTypeImage) && (Word(6) != 0);
215-
}
216-
217208
bool Instruction::IsTensor() const { return (Opcode() == spv::OpTypeTensorARM); }
218209

219210
// Returns "any" constant

‎layers/state_tracker/shader_instruction.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,9 +80,6 @@ class Instruction {
8080
bool IsNonPtrAccessChain() const;
8181
bool IsAccessChain() const;
8282
// Helpers for OpTypeImage
83-
spv::Dim FindImageDim() const;
84-
bool IsImageArray() const;
85-
bool IsImageMultisampled() const;
8683
bool IsTensor() const;
8784
bool IsConstant() const;
8885
bool IsSpecConstant() const;

‎layers/state_tracker/shader_module.cpp‎

Lines changed: 79 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,21 +1937,6 @@ VkFormat Module::GetTensorFormat(const spirv::Instruction& type_inst) const {
19371937
return VK_FORMAT_UNDEFINED;
19381938
}
19391939

1940-
bool Module::HasRuntimeArray(uint32_t type_id) const {
1941-
const Instruction* type = FindDef(type_id);
1942-
if (!type) {
1943-
return false;
1944-
}
1945-
while (type->IsArray() || type->Opcode() == spv::OpTypePointer || type->Opcode() == spv::OpTypeSampledImage) {
1946-
if (type->Opcode() == spv::OpTypeRuntimeArray) {
1947-
return true;
1948-
}
1949-
const uint32_t next_word = (type->Opcode() == spv::OpTypePointer) ? 3 : 2;
1950-
type = FindDef(type->Word(next_word));
1951-
}
1952-
return false;
1953-
}
1954-
19551940
std::string InterfaceSlot::Describe() const {
19561941
std::ostringstream msg;
19571942
msg << "Location = " << Location() << " | Component = " << Component() << " | Type = " << string_SpvOpcode(type) << " "
@@ -2391,21 +2376,20 @@ const Instruction& ResourceInterfaceVariable::FindBaseType(ResourceInterfaceVari
23912376
// Takes a OpVariable and looks at the the descriptor type it uses. This will find things such as if the variable is writable,
23922377
// image atomic operation, matching images to samplers, etc
23932378
const Instruction* type = module_state.FindDef(variable.type_id);
2379+
assert(type->Opcode() == spv::OpTypePointer || type->Opcode() == spv::OpTypeUntypedPointerKHR);
23942380

2395-
// Strip off any array or ptrs. Where we remove array levels, adjust the descriptor count for each dimension.
2396-
while (type->IsArray() || type->Opcode() == spv::OpTypePointer || type->Opcode() == spv::OpTypeSampledImage) {
2397-
if (type->IsArray() || type->Opcode() == spv::OpTypeSampledImage) {
2398-
// currently just tracks 1D arrays
2399-
if (type->Opcode() == spv::OpTypeArray && variable.array_length == 0) {
2400-
variable.array_length = module_state.GetConstantValueById(type->Word(3));
2401-
} else if (type->Opcode() == spv::OpTypeRuntimeArray) {
2402-
variable.array_length = spirv::kRuntimeArray;
2403-
}
2404-
2405-
if (type->Opcode() == spv::OpTypeSampledImage) {
2406-
variable.is_type_sampled_image = true;
2407-
}
2381+
if (variable.data_type_id != 0) {
2382+
type = module_state.FindDef(variable.data_type_id);
2383+
}
24082384

2385+
// Strip off any array or ptrs. Where we remove array levels, adjust the descriptor count for each dimension.
2386+
while (type->IsArray() || type->Opcode() == spv::OpTypePointer) {
2387+
// currently just tracks 1D arrays
2388+
if (type->Opcode() == spv::OpTypeArray && variable.array_length == 0) {
2389+
variable.array_length = module_state.GetConstantValueById(type->Word(3));
2390+
type = module_state.FindDef(type->Word(2)); // Element type
2391+
} else if (type->Opcode() == spv::OpTypeRuntimeArray) {
2392+
variable.array_length = spirv::kRuntimeArray;
24092393
type = module_state.FindDef(type->Word(2)); // Element type
24102394
} else {
24112395
type = module_state.FindDef(type->Word(3)); // Pointer type
@@ -2414,54 +2398,64 @@ const Instruction& ResourceInterfaceVariable::FindBaseType(ResourceInterfaceVari
24142398
return *type;
24152399
}
24162400

2417-
bool ResourceInterfaceVariable::IsStorageBuffer(const ResourceInterfaceVariable& variable) {
2418-
// before VK_KHR_storage_buffer_storage_class Storage Buffer were a Uniform storage class
2419-
const bool physical_storage_buffer = variable.storage_class == spv::StorageClassPhysicalStorageBuffer;
2420-
const bool storage_buffer = variable.storage_class == spv::StorageClassStorageBuffer;
2421-
const bool uniform = variable.storage_class == spv::StorageClassUniform;
2422-
// Block decorations are always on the struct of the variable
2423-
const bool buffer_block =
2424-
variable.type_struct_info && variable.type_struct_info->decorations.Has(DecorationSet::buffer_block_bit);
2425-
const bool block = variable.type_struct_info && variable.type_struct_info->decorations.Has(DecorationSet::block_bit);
2426-
return ((uniform && buffer_block) || ((storage_buffer || physical_storage_buffer) && block));
2427-
}
2428-
2429-
bool ResourceInterfaceVariable::IsUniformBuffer(const ResourceInterfaceVariable& variable) {
2430-
const bool uniform = variable.storage_class == spv::StorageClassUniform;
2431-
const bool block = variable.type_struct_info && variable.type_struct_info->decorations.Has(DecorationSet::block_bit);
2432-
return (uniform && block);
2433-
}
2434-
24352401
ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state, const EntryPoint& entrypoint,
24362402
const Instruction& insn, const ParsedInfo& parsed)
2437-
: VariableBase(module_state, insn, entrypoint.stage, parsed),
2438-
is_type_sampled_image(false),
2439-
base_type(FindBaseType(*this, module_state)),
2440-
is_runtime_descriptor_array(module_state.HasRuntimeArray(type_id)),
2441-
is_storage_buffer(IsStorageBuffer(*this)),
2442-
is_uniform_buffer(IsUniformBuffer(*this)) {
2403+
: VariableBase(module_state, insn, entrypoint.stage, parsed), base_type(FindBaseType(*this, module_state)) {
24432404
// to make sure no padding in-between the struct produce noise and force same data to become a different hash
24442405
info = {}; // will be cleared with c++11 initialization
2445-
info.image_dim = base_type.FindImageDim();
2446-
info.is_image_array = base_type.IsImageArray();
2447-
info.is_multisampled = base_type.IsImageMultisampled();
24482406

2449-
// Handle anything specific to the base type
2450-
if (base_type.Opcode() == spv::OpTypeImage) {
2451-
const spirv::Instruction& element_type_instr = *module_state.FindDef(base_type.Word(2));
2407+
const uint32_t base_type_opcode = base_type.Opcode();
2408+
if (base_type_opcode == spv::OpTypeStruct) {
2409+
assert(type_struct_info);
2410+
// Block/BufferBlock are always on the OpTypeStruct
2411+
if (type_struct_info->decorations.Has(DecorationSet::block_bit)) {
2412+
if (storage_class == spv::StorageClassStorageBuffer) {
2413+
is_storage_buffer = true;
2414+
} else {
2415+
is_uniform_buffer = true;
2416+
}
2417+
} else if (type_struct_info->decorations.Has(DecorationSet::buffer_block_bit)) {
2418+
is_buffer_block = true;
2419+
is_storage_buffer = true;
2420+
}
2421+
} else if (base_type_opcode == spv::OpTypeImage || base_type_opcode == spv::OpTypeSampledImage) {
2422+
// OpTypeSamplerImage == CombinedImageSampler, so we want the image information from it still
2423+
const spirv::Instruction& image_type =
2424+
(base_type_opcode == spv::OpTypeImage) ? base_type : *module_state.FindDef(base_type.Word(2));
2425+
2426+
const spirv::Instruction& element_type_instr = *module_state.FindDef(image_type.Word(2));
24522427
info.numeric_type = module_state.GetNumericType(element_type_instr);
24532428
info.bit_width = static_cast<uint8_t>(element_type_instr.GetBitWidth());
2454-
info.vk_format = CompatibleSpirvImageFormat(base_type.Word(8));
2455-
2456-
// Things marked regardless of the image being accessed or not
2457-
const bool is_sampled_without_sampler = base_type.Word(7) == 2; // Word(7) == Sampled
2458-
if (is_sampled_without_sampler) {
2459-
if (info.image_dim == spv::DimSubpassData) {
2460-
is_input_attachment = true;
2429+
info.vk_format = CompatibleSpirvImageFormat(image_type.Word(8));
2430+
info.image_dim = spv::Dim(image_type.Word(3));
2431+
info.is_image_array = image_type.Word(5) != 0;
2432+
// spirv-val makes sure that the MS operand is only non-zero when possible to be Multisampled
2433+
info.is_multisampled = image_type.Word(6) != 0;
2434+
const bool is_sampled_without_sampler = image_type.Word(7) == 2; // Word(7) == Sampled
2435+
2436+
if (base_type_opcode == spv::OpTypeSampledImage) {
2437+
// Slight relaxation for some GLSL historical madness: samplerBuffer doesn't really have a sampler, and a texel
2438+
// buffer descriptor doesn't really provide one. Allow this slight mismatch.
2439+
const uint32_t dim = image_type.Word(3);
2440+
const uint32_t sampled = image_type.Word(7);
2441+
if (dim == spv::DimBuffer && sampled == 1) {
2442+
is_uniform_texel_buffer = true;
2443+
} else {
2444+
is_combined_image_sampler = true;
2445+
}
2446+
} else if (base_type_opcode == spv::OpTypeImage) {
2447+
if (is_sampled_without_sampler) {
2448+
if (info.image_dim == spv::DimSubpassData) {
2449+
is_input_attachment = true;
2450+
} else if (info.image_dim == spv::DimBuffer) {
2451+
is_storage_texel_buffer = true;
2452+
} else {
2453+
is_storage_image = true;
2454+
}
24612455
} else if (info.image_dim == spv::DimBuffer) {
2462-
is_storage_texel_buffer = true;
2456+
is_uniform_texel_buffer = true;
24632457
} else {
2464-
is_storage_image = true;
2458+
is_sampled_image = true;
24652459
}
24662460
}
24672461

@@ -2483,7 +2477,7 @@ ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state,
24832477
access_mask |= image_access.access_mask;
24842478

24852479
const bool is_image_without_format =
2486-
((is_sampled_without_sampler) && (base_type.Word(8) == spv::ImageFormatUnknown));
2480+
((is_sampled_without_sampler) && (image_type.Word(8) == spv::ImageFormatUnknown));
24872481
if (image_access.access_mask & AccessBit::image_write) {
24882482
if (is_image_without_format) {
24892483
info.is_write_without_format |= true;
@@ -2517,7 +2511,7 @@ ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state,
25172511
}
25182512

25192513
// if not CombinedImageSampler, need to find all Samplers that were accessed with the image
2520-
if (!image_access.variable_sampler_insn.empty() && !is_type_sampled_image) {
2514+
if (!image_access.variable_sampler_insn.empty() && !is_combined_image_sampler) {
25212515
// if no AccessChain, it is same conceptually as being zero
25222516
// TODO - Handle Spec Constants
25232517
const uint32_t image_index = (image_access.image_access_chain_index != kInvalidValue &&
@@ -2543,15 +2537,29 @@ ResourceInterfaceVariable::ResourceInterfaceVariable(const Module& module_state,
25432537
}
25442538
}
25452539
}
2546-
} else if (base_type.Opcode() == spv::OpTypeTensorARM) {
2540+
} else if (base_type_opcode == spv::OpTypeTensorARM) {
25472541
is_storage_tensor = true;
25482542
const spirv::Instruction& element_type_instr = *module_state.FindDef(base_type.Word(2));
25492543
info.numeric_type = module_state.GetNumericType(element_type_instr);
25502544
info.bit_width = static_cast<uint8_t>(element_type_instr.GetBitWidth());
25512545
info.vk_format = module_state.GetTensorFormat(element_type_instr);
25522546
info.tensor_rank = module_state.GetConstantValueById(base_type.Word(3));
2553-
} else if (base_type.Opcode() == spv::OpTypeSampler) {
2547+
} else if (base_type_opcode == spv::OpTypeSampler) {
25542548
is_sampler = true;
2549+
} else if (base_type_opcode == spv::OpTypeAccelerationStructureKHR) {
2550+
// The SPIR-V OpType* are alias, but the Descriptor Types are different
2551+
2552+
// Only KHR or NV base acceleration structure is selected
2553+
if (module_state.HasCapability(spv::CapabilityRayTracingNV)) {
2554+
is_acceleration_structure_nv = true;
2555+
} else {
2556+
is_acceleration_structure = true;
2557+
}
2558+
2559+
// Additionally allow PTLAS if shader uses cluster acceleration structure features
2560+
if (module_state.HasCapability(spv::CapabilityRayTracingClusterAccelerationStructureNV)) {
2561+
is_partitioned_acceleration_structure = true;
2562+
}
25552563
}
25562564

25572565
for (const auto& accessible_id : entrypoint.accessible_ids) {

0 commit comments

Comments
 (0)