Skip to content

Commit

Permalink
Fix an issue of spirv_type used in local variable definitions
Browse files Browse the repository at this point in the history 
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().
  • Loading branch information
@amdrexu
amdrexu committed Nov 18, 2021
1 parent 8a7860e commit bee91eb
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 16 deletions.
25 changes: 11 additions & 14 deletions SPIRV/GlslangToSpv.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4148,23 +4148,23 @@ spv::Id TGlslangToSpvTraverser::convertGlslangToSpvType(const glslang::TType& ty
const auto& spirvType = type.getSpirvType();
const auto& spirvInst = spirvType.spirvInst;

std::vector<spv::Id> operands;
std::vector<spv::IdImmediate> operands;
for (const auto& typeParam : spirvType.typeParams) {
// Constant expression
if (typeParam.constant->isLiteral()) {
if (typeParam.constant->getBasicType() == glslang::EbtFloat) {
float floatValue = static_cast<float>(typeParam.constant->getConstArray()[0].getDConst());
unsigned literal = *reinterpret_cast<unsigned*>(&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;
Expand All @@ -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<char*>(&literal);
charCount = 0;
}
Expand All @@ -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<spv::Op>(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<spv::Op>(spirvInst.id), operands);

break;
}
#endif
Expand Down
31 changes: 31 additions & 0 deletions SPIRV/SpvBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<spv::IdImmediate>& 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<Instruction>(type));
module.mapInstruction(type);

return type->getResultId();
}

// TODO: performance: track arrays per stride
// If a stride is supplied (non-zero) make an array.
Expand Down
1 change: 1 addition & 0 deletions SPIRV/SpvBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ class Builder {
Id makeSamplerType();
Id makeSampledImageType(Id imageType);
Id makeCooperativeMatrixType(Id component, Id scope, Id rows, Id cols);
Id makeGenericType(spv::Op opcode, std::vector<spv::IdImmediate>& operands);

// accelerationStructureNV type
Id makeAccelerationStructureType();
Expand Down
4 changes: 2 additions & 2 deletions Test/baseResults/spv.intrinsicsSpirvType.rgen.out
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions Test/baseResults/spv.intrinsicsSpirvTypeLocalVar.vert.out
View file
Original file line number Diff line number Diff line change
@@ -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
14 changes: 14 additions & 0 deletions Test/spv.intrinsicsSpirvTypeLocalVar.vert
View file
Original file line number Diff line number Diff line change
@@ -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]);
}
1 change: 1 addition & 0 deletions gtests/Spv.FromFile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit bee91eb

Please sign in to comment.