From bee91eb479ee38ce4f268c363154e5175c7a9bd0 Mon Sep 17 00:00:00 2001 From: Rex Xu Date: Wed, 17 Nov 2021 23:32:27 +0800 Subject: [PATCH] Fix an issue of spirv_type used in local variable definitions We previously use createOp() in SPV builder to create type declaration. However, all type declarations should be placed in const-type-variable declaration section. And duplicated type defintions ought to be avoided. We now make a method in SPV builder to perform this operation with a more general solution: makeGenericType(). --- SPIRV/GlslangToSpv.cpp | 25 +++++------ SPIRV/SpvBuilder.cpp | 31 +++++++++++++ SPIRV/SpvBuilder.h | 1 + .../spv.intrinsicsSpirvType.rgen.out | 4 +- .../spv.intrinsicsSpirvTypeLocalVar.vert.out | 43 +++++++++++++++++++ Test/spv.intrinsicsSpirvTypeLocalVar.vert | 14 ++++++ gtests/Spv.FromFile.cpp | 1 + 7 files changed, 103 insertions(+), 16 deletions(-) create mode 100644 Test/baseResults/spv.intrinsicsSpirvTypeLocalVar.vert.out create mode 100644 Test/spv.intrinsicsSpirvTypeLocalVar.vert diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 43aba9cb4..3a906189a 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -4148,23 +4148,23 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty const auto& spirvType = type.getSpirvType(); const auto& spirvInst = spirvType.spirvInst; - std::vector operands; + std::vector operands; for (const auto& typeParam : spirvType.typeParams) { // Constant expression if (typeParam.constant->isLiteral()) { if (typeParam.constant->getBasicType() == glslang::EbtFloat) { float floatValue = static_cast(typeParam.constant->getConstArray()[0].getDConst()); unsigned literal = *reinterpret_cast(&floatValue); - operands.push_back(literal); + operands.push_back({false, literal}); } else if (typeParam.constant->getBasicType() == glslang::EbtInt) { unsigned literal = typeParam.constant->getConstArray()[0].getIConst(); - operands.push_back(literal); + operands.push_back({false, literal}); } else if (typeParam.constant->getBasicType() == glslang::EbtUint) { unsigned literal = typeParam.constant->getConstArray()[0].getUConst(); - operands.push_back(literal); + operands.push_back({false, literal}); } else if (typeParam.constant->getBasicType() == glslang::EbtBool) { unsigned literal = typeParam.constant->getConstArray()[0].getBConst(); - operands.push_back(literal); + operands.push_back({false, literal}); } else if (typeParam.constant->getBasicType() == glslang::EbtString) { auto str = typeParam.constant->getConstArray()[0].getSConst()->c_str(); unsigned literal = 0; @@ -4176,7 +4176,7 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty *(literalPtr++) = ch; ++charCount; if (charCount == 4) { - operands.push_back(literal); + operands.push_back({false, literal}); literalPtr = reinterpret_cast(&literal); charCount = 0; } @@ -4186,20 +4186,17 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty if (charCount > 0) { for (; charCount < 4; ++charCount) *(literalPtr++) = 0; - operands.push_back(literal); + operands.push_back({false, literal}); } } else assert(0); // Unexpected type } else - operands.push_back(createSpvConstant(*typeParam.constant)); + operands.push_back({true, createSpvConstant(*typeParam.constant)}); } - if (spirvInst.set == "") - spvType = builder.createOp(static_cast(spirvInst.id), spv::NoType, operands); - else { - spvType = builder.createBuiltinCall( - spv::NoType, getExtBuiltins(spirvInst.set.c_str()), spirvInst.id, operands); - } + assert(spirvInst.set == ""); // Currently, couldn't be extended instructions. + spvType = builder.makeGenericType(static_cast(spirvInst.id), operands); + break; } #endif diff --git a/SPIRV/SpvBuilder.cpp b/SPIRV/SpvBuilder.cpp index e83306ebc..838f916a0 100644 --- a/SPIRV/SpvBuilder.cpp +++ b/SPIRV/SpvBuilder.cpp @@ -427,6 +427,37 @@ Id Builder::makeCooperativeMatrixType(Id component, Id scope, Id rows, Id cols) return type->getResultId(); } +Id Builder::makeGenericType(spv::Op opcode, std::vector& operands) +{ + // try to find it + Instruction* type; + for (int t = 0; t < (int)groupedTypes[opcode].size(); ++t) { + type = groupedTypes[opcode][t]; + if (type->getNumOperands() != operands.size()) + continue; // Number mismatch, find next + + bool match = true; + for (int op = 0; match && op < operands.size(); ++op) { + match = (operands[op].isId ? type->getIdOperand(op) : type->getImmediateOperand(op)) == operands[op].word; + } + if (match) + return type->getResultId(); + } + + // not found, make it + type = new Instruction(getUniqueId(), NoType, opcode); + for (int op = 0; op < operands.size(); ++op) { + if (operands[op].isId) + type->addIdOperand(operands[op].word); + else + type->addImmediateOperand(operands[op].word); + } + groupedTypes[opcode].push_back(type); + constantsTypesGlobals.push_back(std::unique_ptr(type)); + module.mapInstruction(type); + + return type->getResultId(); +} // TODO: performance: track arrays per stride // If a stride is supplied (non-zero) make an array. diff --git a/SPIRV/SpvBuilder.h b/SPIRV/SpvBuilder.h index 251b9ee82..c72d9b287 100644 --- a/SPIRV/SpvBuilder.h +++ b/SPIRV/SpvBuilder.h @@ -181,6 +181,7 @@ public: Id makeSamplerType(); Id makeSampledImageType(Id imageType); Id makeCooperativeMatrixType(Id component, Id scope, Id rows, Id cols); + Id makeGenericType(spv::Op opcode, std::vector& operands); // accelerationStructureNV type Id makeAccelerationStructureType(); diff --git a/Test/baseResults/spv.intrinsicsSpirvType.rgen.out b/Test/baseResults/spv.intrinsicsSpirvType.rgen.out index ac0c9464a..021e7a6db 100644 --- a/Test/baseResults/spv.intrinsicsSpirvType.rgen.out +++ b/Test/baseResults/spv.intrinsicsSpirvType.rgen.out @@ -22,7 +22,9 @@ Validation failed Decorate 11(as) Binding 0 2: TypeVoid 3: TypeFunction 2 + 6: TypeRayQueryKHR 7: TypePointer Function 6 + 9: TypeAccelerationStructureKHR 10: TypePointer UniformConstant 9 11(as): 10(ptr) Variable UniformConstant 13: TypeInt 32 0 @@ -36,8 +38,6 @@ Validation failed 4(main): 2 Function None 3 5: Label 8(rq): 7(ptr) Variable Function - 6: TypeRayQueryKHR - 9: TypeAccelerationStructureKHR 12: 9 Load 11(as) RayQueryInitializeKHR 8(rq) 12 14 14 18 17 20 19 RayQueryTerminateKHR 8(rq) diff --git a/Test/baseResults/spv.intrinsicsSpirvTypeLocalVar.vert.out b/Test/baseResults/spv.intrinsicsSpirvTypeLocalVar.vert.out new file mode 100644 index 000000000..75515be08 --- /dev/null +++ b/Test/baseResults/spv.intrinsicsSpirvTypeLocalVar.vert.out @@ -0,0 +1,43 @@ +spv.intrinsicsSpirvTypeLocalVar.vert +// Module Version 10000 +// Generated by (magic number): 8000a +// Id's are bound by 22 + + Capability Shader + 1: ExtInstImport "GLSL.std.450" + MemoryModel Logical GLSL450 + EntryPoint Vertex 4 "main" + Source GLSL 460 + SourceExtension "GL_EXT_spirv_intrinsics" + Name 4 "main" + Name 10 "func(spv-t1;" + Name 9 "emptyStruct" + Name 13 "size" + Name 16 "dummy" + Name 18 "param" + Decorate 13(size) SpecId 9 + 2: TypeVoid + 3: TypeFunction 2 + 6: TypeStruct + 7: TypePointer Function 6(struct) + 8: TypeFunction 2 7(ptr) + 12: TypeInt 32 1 + 13(size): 12(int) SpecConstant 9 + 14: TypeArray 6(struct) 13(size) + 15: TypePointer Function 14 + 17: 12(int) Constant 1 + 4(main): 2 Function None 3 + 5: Label + 16(dummy): 15(ptr) Variable Function + 18(param): 7(ptr) Variable Function + 19: 7(ptr) AccessChain 16(dummy) 17 + 20: 6(struct) Load 19 + Store 18(param) 20 + 21: 2 FunctionCall 10(func(spv-t1;) 18(param) + Return + FunctionEnd +10(func(spv-t1;): 2 Function None 8 + 9(emptyStruct): 7(ptr) FunctionParameter + 11: Label + Return + FunctionEnd diff --git a/Test/spv.intrinsicsSpirvTypeLocalVar.vert b/Test/spv.intrinsicsSpirvTypeLocalVar.vert new file mode 100644 index 000000000..203d90073 --- /dev/null +++ b/Test/spv.intrinsicsSpirvTypeLocalVar.vert @@ -0,0 +1,14 @@ +#version 460 core + +#extension GL_EXT_spirv_intrinsics: enable + +layout(constant_id = 9) const int size = 9; + +#define EmptyStruct spirv_type(id = 30) +void func(EmptyStruct emptyStruct) {} + +void main() +{ + EmptyStruct dummy[size]; + func(dummy[1]); +} diff --git a/gtests/Spv.FromFile.cpp b/gtests/Spv.FromFile.cpp index ef1176404..460c1d220 100644 --- a/gtests/Spv.FromFile.cpp +++ b/gtests/Spv.FromFile.cpp @@ -373,6 +373,7 @@ INSTANTIATE_TEST_SUITE_P( "spv.intrinsicsSpirvLiteral.vert", "spv.intrinsicsSpirvStorageClass.rchit", "spv.intrinsicsSpirvType.rgen", + "spv.intrinsicsSpirvTypeLocalVar.vert", "spv.invariantAll.vert", "spv.layer.tese", "spv.layoutNested.vert",