diff --git a/3rdparty/spirv-tools/include/generated/build-version.inc b/3rdparty/spirv-tools/include/generated/build-version.inc index 5a30d3ef8..5921ced90 100644 --- a/3rdparty/spirv-tools/include/generated/build-version.inc +++ b/3rdparty/spirv-tools/include/generated/build-version.inc @@ -1 +1 @@ -"v2021.4-dev", "SPIRV-Tools v2021.4-dev 7e0cccc269f6476f79172f4446ab902f6c451f17" +"v2021.4-dev", "SPIRV-Tools v2021.4-dev 6d1791ae2cbef975b42b8979526d79514be83329" diff --git a/3rdparty/spirv-tools/include/generated/nonsemantic.clspvreflection.insts.inc b/3rdparty/spirv-tools/include/generated/nonsemantic.clspvreflection.insts.inc index 4f559f6ae..2cf7cafe3 100644 --- a/3rdparty/spirv-tools/include/generated/nonsemantic.clspvreflection.insts.inc +++ b/3rdparty/spirv-tools/include/generated/nonsemantic.clspvreflection.insts.inc @@ -24,5 +24,6 @@ static const spv_ext_inst_desc_t nonsemantic_clspvreflection_entries[] = { {"ConstantDataStorageBuffer", 21, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}}, {"ConstantDataUniform", 22, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}}, {"LiteralSampler", 23, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}}, - {"PropertyRequiredWorkgroupSize", 24, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}} + {"PropertyRequiredWorkgroupSize", 24, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}}, + {"SpecConstantSubgroupMaxSize", 25, 0, nullptr, {SPV_OPERAND_TYPE_ID, SPV_OPERAND_TYPE_NONE}} }; \ No newline at end of file diff --git a/3rdparty/spirv-tools/source/opt/desc_sroa.cpp b/3rdparty/spirv-tools/source/opt/desc_sroa.cpp index 0b8393705..bcbdde94c 100644 --- a/3rdparty/spirv-tools/source/opt/desc_sroa.cpp +++ b/3rdparty/spirv-tools/source/opt/desc_sroa.cpp @@ -19,6 +19,14 @@ namespace spvtools { namespace opt { +namespace { + +bool IsDecorationBinding(Instruction* inst) { + if (inst->opcode() != SpvOpDecorate) return false; + return inst->GetSingleWordInOperand(1u) == SpvDecorationBinding; +} + +} // namespace Pass::Status DescriptorScalarReplacement::Process() { bool modified = false; @@ -157,6 +165,74 @@ uint32_t DescriptorScalarReplacement::GetReplacementVariable(Instruction* var, return replacement_vars->second[idx]; } +void DescriptorScalarReplacement::CopyDecorationsForNewVariable( + Instruction* old_var, uint32_t index, uint32_t new_var_id, + uint32_t new_var_ptr_type_id, const bool is_old_var_array, + const bool is_old_var_struct, Instruction* old_var_type) { + // Handle OpDecorate instructions. + for (auto old_decoration : + get_decoration_mgr()->GetDecorationsFor(old_var->result_id(), true)) { + uint32_t new_binding = 0; + if (IsDecorationBinding(old_decoration)) { + new_binding = GetNewBindingForElement( + old_decoration->GetSingleWordInOperand(2), index, new_var_ptr_type_id, + is_old_var_array, is_old_var_struct, old_var_type); + } + CreateNewDecorationForNewVariable(old_decoration, new_var_id, new_binding); + } + + // Handle OpMemberDecorate instructions. + for (auto old_decoration : get_decoration_mgr()->GetDecorationsFor( + old_var_type->result_id(), true)) { + assert(old_decoration->opcode() == SpvOpMemberDecorate); + if (old_decoration->GetSingleWordInOperand(1u) != index) continue; + CreateNewDecorationForMemberDecorate(old_decoration, new_var_id); + } +} + +uint32_t DescriptorScalarReplacement::GetNewBindingForElement( + uint32_t old_binding, uint32_t index, uint32_t new_var_ptr_type_id, + const bool is_old_var_array, const bool is_old_var_struct, + Instruction* old_var_type) { + if (is_old_var_array) { + return old_binding + index * GetNumBindingsUsedByType(new_var_ptr_type_id); + } + if (is_old_var_struct) { + // The binding offset that should be added is the sum of binding + // numbers used by previous members of the current struct. + uint32_t new_binding = old_binding; + for (uint32_t i = 0; i < index; ++i) { + new_binding += + GetNumBindingsUsedByType(old_var_type->GetSingleWordInOperand(i)); + } + return new_binding; + } + return old_binding; +} + +void DescriptorScalarReplacement::CreateNewDecorationForNewVariable( + Instruction* old_decoration, uint32_t new_var_id, uint32_t new_binding) { + assert(old_decoration->opcode() == SpvOpDecorate); + std::unique_ptr new_decoration(old_decoration->Clone(context())); + new_decoration->SetInOperand(0, {new_var_id}); + + if (IsDecorationBinding(new_decoration.get())) { + new_decoration->SetInOperand(2, {new_binding}); + } + context()->AddAnnotationInst(std::move(new_decoration)); +} + +void DescriptorScalarReplacement::CreateNewDecorationForMemberDecorate( + Instruction* old_member_decoration, uint32_t new_var_id) { + std::vector operands( + {{spv_operand_type_t::SPV_OPERAND_TYPE_ID, {new_var_id}}}); + auto new_decorate_operand_begin = old_member_decoration->begin() + 2u; + auto new_decorate_operand_end = old_member_decoration->end(); + operands.insert(operands.end(), new_decorate_operand_begin, + new_decorate_operand_end); + get_decoration_mgr()->AddDecoration(SpvOpDecorate, std::move(operands)); +} + uint32_t DescriptorScalarReplacement::CreateReplacementVariable( Instruction* var, uint32_t idx) { // The storage class for the new variable is the same as the original. @@ -192,33 +268,8 @@ uint32_t DescriptorScalarReplacement::CreateReplacementVariable( {static_cast(storage_class)}}})); context()->AddGlobalValue(std::move(variable)); - // Copy all of the decorations to the new variable. The only difference is - // the Binding decoration needs to be adjusted. - for (auto old_decoration : - get_decoration_mgr()->GetDecorationsFor(var->result_id(), true)) { - assert(old_decoration->opcode() == SpvOpDecorate); - std::unique_ptr new_decoration( - old_decoration->Clone(context())); - new_decoration->SetInOperand(0, {id}); - - uint32_t decoration = new_decoration->GetSingleWordInOperand(1u); - if (decoration == SpvDecorationBinding) { - uint32_t new_binding = new_decoration->GetSingleWordInOperand(2); - if (is_array) { - new_binding += idx * GetNumBindingsUsedByType(ptr_element_type_id); - } - if (is_struct) { - // The binding offset that should be added is the sum of binding numbers - // used by previous members of the current struct. - for (uint32_t i = 0; i < idx; ++i) { - new_binding += GetNumBindingsUsedByType( - pointee_type_inst->GetSingleWordInOperand(i)); - } - } - new_decoration->SetInOperand(2, {new_binding}); - } - context()->AddAnnotationInst(std::move(new_decoration)); - } + CopyDecorationsForNewVariable(var, idx, id, ptr_element_type_id, is_array, + is_struct, pointee_type_inst); // Create a new OpName for the replacement variable. std::vector> names_to_add; diff --git a/3rdparty/spirv-tools/source/opt/desc_sroa.h b/3rdparty/spirv-tools/source/opt/desc_sroa.h index 70fd381af..fea062559 100644 --- a/3rdparty/spirv-tools/source/opt/desc_sroa.h +++ b/3rdparty/spirv-tools/source/opt/desc_sroa.h @@ -89,6 +89,46 @@ class DescriptorScalarReplacement : public Pass { // bindings used by its members. uint32_t GetNumBindingsUsedByType(uint32_t type_id); + // Copy all of the decorations of variable |old_var| and make them as + // decorations for the new variable whose id is |new_var_id|. The new variable + // is supposed to replace |index|th element of |old_var|. + // |new_var_ptr_type_id| is the id of the pointer to the type of the new + // variable. |is_old_var_array| is true if |old_var| has an array type. + // |is_old_var_struct| is true if |old_var| has a structure type. + // |old_var_type| is the pointee type of |old_var|. + void CopyDecorationsForNewVariable(Instruction* old_var, uint32_t index, + uint32_t new_var_id, + uint32_t new_var_ptr_type_id, + const bool is_old_var_array, + const bool is_old_var_struct, + Instruction* old_var_type); + + // Get the new binding number for a new variable that will be replaced with an + // |index|th element of an old variable. The old variable has |old_binding| + // as its binding number. |ptr_elem_type_id| the id of the pointer to the + // element type. |is_old_var_array| is true if the old variable has an array + // type. |is_old_var_struct| is true if the old variable has a structure type. + // |old_var_type| is the pointee type of the old variable. + uint32_t GetNewBindingForElement(uint32_t old_binding, uint32_t index, + uint32_t ptr_elem_type_id, + const bool is_old_var_array, + const bool is_old_var_struct, + Instruction* old_var_type); + + // Create a new OpDecorate instruction by cloning |old_decoration|. The new + // OpDecorate instruction will be used for a variable whose id is + // |new_var_ptr_type_id|. If |old_decoration| is a decoration for a binding, + // the new OpDecorate instruction will have |new_binding| as its binding. + void CreateNewDecorationForNewVariable(Instruction* old_decoration, + uint32_t new_var_id, + uint32_t new_binding); + + // Create a new OpDecorate instruction whose operand is the same as an + // OpMemberDecorate instruction |old_member_decoration| except Target operand. + // The Target operand of the new OpDecorate instruction will be |new_var_id|. + void CreateNewDecorationForMemberDecorate(Instruction* old_decoration, + uint32_t new_var_id); + // A map from an OpVariable instruction to the set of variables that will be // used to replace it. The entry |replacement_variables_[var][i]| is the id of // a variable that will be used in the place of the the ith element of the diff --git a/3rdparty/spirv-tools/source/opt/ir_builder.h b/3rdparty/spirv-tools/source/opt/ir_builder.h index fe5feff56..4433cf0d1 100644 --- a/3rdparty/spirv-tools/source/opt/ir_builder.h +++ b/3rdparty/spirv-tools/source/opt/ir_builder.h @@ -359,8 +359,9 @@ class InstructionBuilder { return AddInstruction(std::move(select)); } - // Adds a signed int32 constant to the binary. - // The |value| parameter is the constant value to be added. + // Returns a pointer to the definition of a signed 32-bit integer constant + // with the given value. Returns |nullptr| if the constant does not exist and + // cannot be created. Instruction* GetSintConstant(int32_t value) { return GetIntConstant(value, true); } @@ -381,21 +382,24 @@ class InstructionBuilder { GetContext()->TakeNextId(), ops)); return AddInstruction(std::move(construct)); } - // Adds an unsigned int32 constant to the binary. - // The |value| parameter is the constant value to be added. + + // Returns a pointer to the definition of an unsigned 32-bit integer constant + // with the given value. Returns |nullptr| if the constant does not exist and + // cannot be created. Instruction* GetUintConstant(uint32_t value) { return GetIntConstant(value, false); } uint32_t GetUintConstantId(uint32_t value) { Instruction* uint_inst = GetUintConstant(value); - return uint_inst->result_id(); + return (uint_inst != nullptr ? uint_inst->result_id() : 0); } // Adds either a signed or unsigned 32 bit integer constant to the binary - // depedning on the |sign|. If |sign| is true then the value is added as a + // depending on the |sign|. If |sign| is true then the value is added as a // signed constant otherwise as an unsigned constant. If |sign| is false the - // value must not be a negative number. + // value must not be a negative number. Returns false if the constant does + // not exists and could be be created. template Instruction* GetIntConstant(T value, bool sign) { // Assert that we are not trying to store a negative number in an unsigned @@ -411,6 +415,10 @@ class InstructionBuilder { uint32_t type_id = GetContext()->get_type_mgr()->GetTypeInstruction(&int_type); + if (type_id == 0) { + return nullptr; + } + // Get the memory managed type so that it is safe to be stored by // GetConstant. analysis::Type* rebuilt_type = diff --git a/3rdparty/spirv-tools/source/opt/merge_return_pass.cpp b/3rdparty/spirv-tools/source/opt/merge_return_pass.cpp index f1601049e..a962a7ccb 100644 --- a/3rdparty/spirv-tools/source/opt/merge_return_pass.cpp +++ b/3rdparty/spirv-tools/source/opt/merge_return_pass.cpp @@ -111,7 +111,9 @@ bool MergeReturnPass::ProcessStructured( } RecordImmediateDominators(function); - AddSingleCaseSwitchAroundFunction(); + if (!AddSingleCaseSwitchAroundFunction()) { + return false; + } std::list order; cfg()->ComputeStructuredOrder(function, &*function->begin(), &order); @@ -770,7 +772,7 @@ void MergeReturnPass::InsertAfterElement(BasicBlock* element, list->insert(pos, new_element); } -void MergeReturnPass::AddSingleCaseSwitchAroundFunction() { +bool MergeReturnPass::AddSingleCaseSwitchAroundFunction() { CreateReturnBlock(); CreateReturn(final_return_block_); @@ -778,7 +780,10 @@ void MergeReturnPass::AddSingleCaseSwitchAroundFunction() { cfg()->RegisterBlock(final_return_block_); } - CreateSingleCaseSwitch(final_return_block_); + if (!CreateSingleCaseSwitch(final_return_block_)) { + return false; + } + return true; } BasicBlock* MergeReturnPass::CreateContinueTarget(uint32_t header_label_id) { @@ -813,7 +818,7 @@ BasicBlock* MergeReturnPass::CreateContinueTarget(uint32_t header_label_id) { return new_block; } -void MergeReturnPass::CreateSingleCaseSwitch(BasicBlock* merge_target) { +bool MergeReturnPass::CreateSingleCaseSwitch(BasicBlock* merge_target) { // Insert the switch before any code is run. We have to split the entry // block to make sure the OpVariable instructions remain in the entry block. BasicBlock* start_block = &*function_->begin(); @@ -830,13 +835,17 @@ void MergeReturnPass::CreateSingleCaseSwitch(BasicBlock* merge_target) { context(), start_block, IRContext::kAnalysisDefUse | IRContext::kAnalysisInstrToBlockMapping); - builder.AddSwitch(builder.GetUintConstantId(0u), old_block->id(), {}, - merge_target->id()); + uint32_t const_zero_id = builder.GetUintConstantId(0u); + if (const_zero_id == 0) { + return false; + } + builder.AddSwitch(const_zero_id, old_block->id(), {}, merge_target->id()); if (context()->AreAnalysesValid(IRContext::kAnalysisCFG)) { cfg()->RegisterBlock(old_block); cfg()->AddEdges(start_block); } + return true; } bool MergeReturnPass::HasNontrivialUnreachableBlocks(Function* function) { diff --git a/3rdparty/spirv-tools/source/opt/merge_return_pass.h b/3rdparty/spirv-tools/source/opt/merge_return_pass.h index 06a3e7b55..4096ce7dd 100644 --- a/3rdparty/spirv-tools/source/opt/merge_return_pass.h +++ b/3rdparty/spirv-tools/source/opt/merge_return_pass.h @@ -277,7 +277,7 @@ class MergeReturnPass : public MemPass { // current function where the switch and case value are both zero and the // default is the merge block. Returns after the switch is executed. Sets // |final_return_block_|. - void AddSingleCaseSwitchAroundFunction(); + bool AddSingleCaseSwitchAroundFunction(); // Creates a new basic block that branches to |header_label_id|. Returns the // new basic block. The block will be the second last basic block in the @@ -286,7 +286,7 @@ class MergeReturnPass : public MemPass { // Creates a one case switch around the executable code of the function with // |merge_target| as the merge node. - void CreateSingleCaseSwitch(BasicBlock* merge_target); + bool CreateSingleCaseSwitch(BasicBlock* merge_target); // Returns true if |function| has an unreachable block that is not a continue // target that simply branches back to the header, or a merge block containing diff --git a/3rdparty/spirv-tools/source/val/validate_annotation.cpp b/3rdparty/spirv-tools/source/val/validate_annotation.cpp index 85d2b7515..16d44906d 100644 --- a/3rdparty/spirv-tools/source/val/validate_annotation.cpp +++ b/3rdparty/spirv-tools/source/val/validate_annotation.cpp @@ -138,14 +138,14 @@ std::string LogStringForDecoration(uint32_t decoration) { return "PerTaskNV"; case SpvDecorationPerVertexNV: return "PerVertexNV"; - case SpvDecorationNonUniformEXT: - return "NonUniformEXT"; - case SpvDecorationRestrictPointerEXT: - return "RestrictPointerEXT"; - case SpvDecorationAliasedPointerEXT: - return "AliasedPointerEXT"; - case SpvDecorationHlslCounterBufferGOOGLE: - return "HlslCounterBufferGOOGLE"; + case SpvDecorationNonUniform: + return "NonUniform"; + case SpvDecorationRestrictPointer: + return "RestrictPointer"; + case SpvDecorationAliasedPointer: + return "AliasedPointer"; + case SpvDecorationCounterBuffer: + return "CounterBuffer"; case SpvDecorationHlslSemanticGOOGLE: return "HlslSemanticGOOGLE"; default: @@ -156,8 +156,8 @@ std::string LogStringForDecoration(uint32_t decoration) { // Returns true if the decoration takes ID parameters. // TODO(dneto): This can be generated from the grammar. -bool DecorationTakesIdParameters(uint32_t type) { - switch (static_cast(type)) { +bool DecorationTakesIdParameters(SpvDecoration type) { + switch (type) { case SpvDecorationUniformId: case SpvDecorationAlignmentId: case SpvDecorationMaxByteOffsetId: @@ -169,17 +169,212 @@ bool DecorationTakesIdParameters(uint32_t type) { return false; } -spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) { - const auto decoration = inst->GetOperandAs(1); - if (decoration == SpvDecorationSpecId) { - const auto target_id = inst->GetOperandAs(0); - const auto target = _.FindDef(target_id); - if (!target || !spvOpcodeIsScalarSpecConstant(target->opcode())) { - return _.diag(SPV_ERROR_INVALID_ID, inst) - << "OpDecorate SpecId decoration target '" - << _.getIdName(target_id) - << "' is not a scalar specialization constant."; +bool IsMemberDecorationOnly(SpvDecoration dec) { + switch (dec) { + case SpvDecorationRowMajor: + case SpvDecorationColMajor: + case SpvDecorationMatrixStride: + // SPIR-V spec bug? Offset is generated on variables when dealing with + // transform feedback. + // case SpvDecorationOffset: + return true; + default: + break; + } + return false; +} + +bool IsNotMemberDecoration(SpvDecoration dec) { + switch (dec) { + case SpvDecorationSpecId: + case SpvDecorationBlock: + case SpvDecorationBufferBlock: + case SpvDecorationArrayStride: + case SpvDecorationGLSLShared: + case SpvDecorationGLSLPacked: + case SpvDecorationCPacked: + // TODO: https://github.com/KhronosGroup/glslang/issues/703: + // glslang applies Restrict to structure members. + // case SpvDecorationRestrict: + case SpvDecorationAliased: + case SpvDecorationConstant: + case SpvDecorationUniform: + case SpvDecorationUniformId: + case SpvDecorationSaturatedConversion: + case SpvDecorationIndex: + case SpvDecorationBinding: + case SpvDecorationDescriptorSet: + case SpvDecorationFuncParamAttr: + case SpvDecorationFPRoundingMode: + case SpvDecorationFPFastMathMode: + case SpvDecorationLinkageAttributes: + case SpvDecorationNoContraction: + case SpvDecorationInputAttachmentIndex: + case SpvDecorationAlignment: + case SpvDecorationMaxByteOffset: + case SpvDecorationAlignmentId: + case SpvDecorationMaxByteOffsetId: + case SpvDecorationNoSignedWrap: + case SpvDecorationNoUnsignedWrap: + case SpvDecorationNonUniform: + case SpvDecorationRestrictPointer: + case SpvDecorationAliasedPointer: + case SpvDecorationCounterBuffer: + return true; + default: + break; + } + return false; +} + +spv_result_t ValidateDecorationTarget(ValidationState_t& _, SpvDecoration dec, + const Instruction* inst, + const Instruction* target) { + auto fail = [&_, dec, inst, target](uint32_t vuid = 0) -> DiagnosticStream { + DiagnosticStream ds = std::move( + _.diag(SPV_ERROR_INVALID_ID, inst) + << _.VkErrorID(vuid) << LogStringForDecoration(dec) + << " decoration on target '" << _.getIdName(target->id()) << "' "); + return ds; + }; + switch (dec) { + case SpvDecorationSpecId: + if (!spvOpcodeIsScalarSpecConstant(target->opcode())) { + return fail() << "must be a scalar specialization constant"; + } + break; + case SpvDecorationBlock: + case SpvDecorationBufferBlock: + case SpvDecorationGLSLShared: + case SpvDecorationGLSLPacked: + case SpvDecorationCPacked: + if (target->opcode() != SpvOpTypeStruct) { + return fail() << "must be a structure type"; + } + break; + case SpvDecorationArrayStride: + if (target->opcode() != SpvOpTypeArray && + target->opcode() != SpvOpTypeRuntimeArray && + target->opcode() != SpvOpTypePointer) { + return fail() << "must be an array or pointer type"; + } + break; + case SpvDecorationBuiltIn: + if (target->opcode() != SpvOpVariable && + !spvOpcodeIsConstant(target->opcode())) { + return _.diag(SPV_ERROR_INVALID_DATA, inst) + << "BuiltIns can only target variables, structure members or " + "constants"; + } + if (inst->GetOperandAs(2) == SpvBuiltInWorkgroupSize) { + if (!spvOpcodeIsConstant(target->opcode())) { + return fail() << "must be a constant for WorkgroupSize"; + } + } else if (target->opcode() != SpvOpVariable) { + return fail() << "must be a variable"; + } + break; + case SpvDecorationNoPerspective: + case SpvDecorationFlat: + case SpvDecorationPatch: + case SpvDecorationCentroid: + case SpvDecorationSample: + case SpvDecorationRestrict: + case SpvDecorationAliased: + case SpvDecorationVolatile: + case SpvDecorationCoherent: + case SpvDecorationNonWritable: + case SpvDecorationNonReadable: + case SpvDecorationXfbBuffer: + case SpvDecorationXfbStride: + case SpvDecorationComponent: + case SpvDecorationStream: + case SpvDecorationRestrictPointer: + case SpvDecorationAliasedPointer: + if (target->opcode() != SpvOpVariable && + target->opcode() != SpvOpFunctionParameter) { + return fail() << "must be a memory object declaration"; + } + if (_.GetIdOpcode(target->type_id()) != SpvOpTypePointer) { + return fail() << "must be a pointer type"; + } + break; + case SpvDecorationInvariant: + case SpvDecorationConstant: + case SpvDecorationLocation: + case SpvDecorationIndex: + case SpvDecorationBinding: + case SpvDecorationDescriptorSet: + case SpvDecorationInputAttachmentIndex: + if (target->opcode() != SpvOpVariable) { + return fail() << "must be a variable"; + } + break; + default: + break; + } + + if (spvIsVulkanEnv(_.context()->target_env)) { + // The following were all checked as pointer types above. + SpvStorageClass sc = SpvStorageClassUniform; + const auto type = _.FindDef(target->type_id()); + if (type && type->operands().size() > 2) { + sc = type->GetOperandAs(1); } + switch (dec) { + case SpvDecorationLocation: + case SpvDecorationComponent: + // Location is used for input, output and ray tracing stages. + if (sc == SpvStorageClassStorageBuffer || + sc == SpvStorageClassUniform || + sc == SpvStorageClassUniformConstant || + sc == SpvStorageClassWorkgroup || sc == SpvStorageClassPrivate || + sc == SpvStorageClassFunction) { + return _.diag(SPV_ERROR_INVALID_ID, target) + << LogStringForDecoration(dec) + << " decoration must not be applied to this storage class"; + } + break; + case SpvDecorationIndex: + if (sc != SpvStorageClassOutput) { + return fail() << "must be in the Output storage class"; + } + break; + case SpvDecorationBinding: + case SpvDecorationDescriptorSet: + if (sc != SpvStorageClassStorageBuffer && + sc != SpvStorageClassUniform && + sc != SpvStorageClassUniformConstant) { + return fail() << "must be in the StorageBuffer, Uniform, or " + "UniformConstant storage class"; + } + break; + case SpvDecorationInputAttachmentIndex: + if (sc != SpvStorageClassUniformConstant) { + return fail() << "must be in the UniformConstant storage class"; + } + break; + case SpvDecorationFlat: + case SpvDecorationNoPerspective: + case SpvDecorationCentroid: + case SpvDecorationSample: + if (sc != SpvStorageClassInput && sc != SpvStorageClassOutput) { + return fail(4670) << "storage class must be Input or Output"; + } + break; + default: + break; + } + } + return SPV_SUCCESS; +} + +spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) { + const auto decoration = inst->GetOperandAs(1); + const auto target_id = inst->GetOperandAs(0); + const auto target = _.FindDef(target_id); + if (!target) { + return _.diag(SPV_ERROR_INVALID_ID, inst) << "target is not defined"; } if (spvIsVulkanEnv(_.context()->target_env)) { @@ -197,17 +392,34 @@ spv_result_t ValidateDecorate(ValidationState_t& _, const Instruction* inst) { << "Decorations taking ID parameters may not be used with " "OpDecorateId"; } + + if (target->opcode() != SpvOpDecorationGroup) { + if (IsMemberDecorationOnly(decoration)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << LogStringForDecoration(decoration) + << " can only be applied to structure members"; + } + + if (auto error = ValidateDecorationTarget(_, decoration, inst, target)) { + return error; + } + } + // TODO: Add validations for all decorations. return SPV_SUCCESS; } spv_result_t ValidateDecorateId(ValidationState_t& _, const Instruction* inst) { - const auto decoration = inst->GetOperandAs(1); + const auto decoration = inst->GetOperandAs(1); if (!DecorationTakesIdParameters(decoration)) { return _.diag(SPV_ERROR_INVALID_ID, inst) << "Decorations that don't take ID parameters may not be used with " "OpDecorateId"; } + + // No member decorations take id parameters, so we don't bother checking if + // we are using a member only decoration here. + // TODO: Add validations for these decorations. // UniformId is covered elsewhere. return SPV_SUCCESS; @@ -234,6 +446,13 @@ spv_result_t ValidateMemberDecorate(ValidationState_t& _, << " members. Largest valid index is " << member_count - 1 << "."; } + const auto decoration = inst->GetOperandAs(2); + if (IsNotMemberDecoration(decoration)) { + return _.diag(SPV_ERROR_INVALID_ID, inst) + << LogStringForDecoration(decoration) + << " cannot be applied to structure members"; + } + return SPV_SUCCESS; } diff --git a/3rdparty/spirv-tools/source/val/validate_builtins.cpp b/3rdparty/spirv-tools/source/val/validate_builtins.cpp index a6e624f74..57dde8ad9 100644 --- a/3rdparty/spirv-tools/source/val/validate_builtins.cpp +++ b/3rdparty/spirv-tools/source/val/validate_builtins.cpp @@ -3993,14 +3993,6 @@ spv_result_t BuiltInsValidator::ValidateSingleBuiltInAtDefinition( const Decoration& decoration, const Instruction& inst) { const SpvBuiltIn label = SpvBuiltIn(decoration.params()[0]); - // Builtins can only be applied to variables, structures or constants. - auto target_opcode = inst.opcode(); - if (target_opcode != SpvOpTypeStruct && target_opcode != SpvOpVariable && - !spvOpcodeIsConstant(target_opcode)) { - return _.diag(SPV_ERROR_INVALID_DATA, &inst) - << "BuiltIns can only target variables, structs or constants"; - } - if (!spvIsVulkanEnv(_.context()->target_env)) { // Early return. All currently implemented rules are based on Vulkan spec. // diff --git a/3rdparty/spirv-tools/source/val/validation_state.cpp b/3rdparty/spirv-tools/source/val/validation_state.cpp index 796387275..8d1a0d3f4 100644 --- a/3rdparty/spirv-tools/source/val/validation_state.cpp +++ b/3rdparty/spirv-tools/source/val/validation_state.cpp @@ -1839,6 +1839,8 @@ std::string ValidationState_t::VkErrorID(uint32_t id, return VUID_WRAP(VUID-StandaloneSpirv-None-04667); case 4669: return VUID_WRAP(VUID-StandaloneSpirv-GLSLShared-04669); + case 4670: + return VUID_WRAP(VUID-StandaloneSpirv-Flat-04670); case 4675: return VUID_WRAP(VUID-StandaloneSpirv-FPRoundingMode-04675); case 4677: