From 3fd123266584a93372e134223432eece61df2422 Mon Sep 17 00:00:00 2001 From: Jeff Bolz Date: Tue, 5 Mar 2019 23:27:09 -0600 Subject: [PATCH] Improved fix for buffer reference constants This is an alternate fix for the issue described in commit be63facd, whose solution didn't work if there were non-trivial operations involved in computing a constant initializer which caused the 'constant unfolding' code to kick in (addConstantReferenceConversion). Instead, this change does the 'unfolding' later in createSpvConstantFromConstUnionArray. If a reference-type constant has survived that long, then folding is already done, this must be a 'real' (inside a function) use of the constant, and it should be safe to unfold and apply the bitcast. --- SPIRV/GlslangToSpv.cpp | 4 + Test/baseResults/spv.bufferhandle16.frag.out | 118 ++++++++++-------- Test/spv.bufferhandle16.frag | 3 + glslang/MachineIndependent/Intermediate.cpp | 18 --- glslang/MachineIndependent/ParseHelper.cpp | 6 +- .../MachineIndependent/localintermediate.h | 1 - 6 files changed, 75 insertions(+), 75 deletions(-) diff --git a/SPIRV/GlslangToSpv.cpp b/SPIRV/GlslangToSpv.cpp index 5b753ce1..07f30350 100644 --- a/SPIRV/GlslangToSpv.cpp +++ b/SPIRV/GlslangToSpv.cpp @@ -7705,6 +7705,10 @@ spv::Id TGlslangToSpvTraverser::createSpvConstantFromConstUnionArray(const glsla case glslang::EbtBool: scalar = builder.makeBoolConstant(zero ? false : consts[nextConst].getBConst(), specConstant); break; + case glslang::EbtReference: + scalar = builder.makeUint64Constant(zero ? 0 : consts[nextConst].getU64Const(), specConstant); + scalar = builder.createUnaryOp(spv::OpBitcast, typeId, scalar); + break; default: assert(0); break; diff --git a/Test/baseResults/spv.bufferhandle16.frag.out b/Test/baseResults/spv.bufferhandle16.frag.out index 79e5b6b6..a4cf44d1 100644 --- a/Test/baseResults/spv.bufferhandle16.frag.out +++ b/Test/baseResults/spv.bufferhandle16.frag.out @@ -1,7 +1,7 @@ spv.bufferhandle16.frag // Module Version 10000 // Generated by (magic number): 80007 -// Id's are bound by 37 +// Id's are bound by 48 Capability Shader Capability Int64 @@ -16,61 +16,77 @@ spv.bufferhandle16.frag SourceExtension "GL_EXT_scalar_block_layout" SourceExtension "GL_EXT_shader_explicit_arithmetic_types_int64" Name 4 "main" - Name 8 "T1" - MemberName 8(T1) 0 "x" - Name 10 "a" - Name 14 "b" - Name 17 "c" - Name 23 "d" - Name 25 "e" - Name 36 "x" - MemberDecorate 8(T1) 0 Offset 0 - Decorate 8(T1) Block - Decorate 10(a) DecorationAliasedPointerEXT - Decorate 14(b) DecorationAliasedPointerEXT - Decorate 17(c) DecorationAliasedPointerEXT - Decorate 23(d) DecorationAliasedPointerEXT - Decorate 25(e) DecorationAliasedPointerEXT + Name 9 "T1" + MemberName 9(T1) 0 "x" + MemberName 9(T1) 1 "y" + Name 11 "a" + Name 15 "b" + Name 18 "c" + Name 24 "d" + Name 26 "e" + Name 29 "f" + Name 46 "x" + MemberDecorate 9(T1) 0 Offset 0 + MemberDecorate 9(T1) 1 Offset 4 + Decorate 9(T1) Block + Decorate 11(a) DecorationAliasedPointerEXT + Decorate 15(b) DecorationAliasedPointerEXT + Decorate 18(c) DecorationAliasedPointerEXT + Decorate 24(d) DecorationAliasedPointerEXT + Decorate 26(e) DecorationAliasedPointerEXT + Decorate 29(f) DecorationAliasedPointerEXT 2: TypeVoid 3: TypeFunction 2 TypeForwardPointer 6 PhysicalStorageBufferEXT 7: TypeInt 32 1 - 8(T1): TypeStruct 7(int) - 6: TypePointer PhysicalStorageBufferEXT 8(T1) - 9: TypePointer Function 6(ptr) - 11: TypeInt 64 0 - 12: 11(int64_t) Constant 4 0 - 15: 11(int64_t) Constant 5 0 - 18: TypeBool - 19: 18(bool) ConstantTrue - 26: 11(int64_t) Constant 6 0 - 28: 11(int64_t) Constant 7 0 - 31: 7(int) Constant 3 - 32: TypeInt 32 0 - 33: 32(int) Constant 3 - 34: TypeArray 7(int) 33 - 35: TypePointer Private 34 - 36(x): 35(ptr) Variable Private + 8: TypeInt 32 0 + 9(T1): TypeStruct 7(int) 8(int) + 6: TypePointer PhysicalStorageBufferEXT 9(T1) + 10: TypePointer Function 6(ptr) + 12: TypeInt 64 0 + 13: 12(int64_t) Constant 4 0 + 16: 12(int64_t) Constant 5 0 + 19: TypeBool + 20: 19(bool) ConstantTrue + 27: 12(int64_t) Constant 6 0 + 31: 7(int) Constant 1 + 32: TypePointer PhysicalStorageBufferEXT 8(int) + 35: 8(int) Constant 0 + 37: 12(int64_t) Constant 8 0 + 39: 12(int64_t) Constant 9 0 + 42: 7(int) Constant 3 + 43: 8(int) Constant 3 + 44: TypeArray 7(int) 43 + 45: TypePointer Private 44 + 46(x): 45(ptr) Variable Private + 47: 12(int64_t) Constant 10 0 4(main): 2 Function None 3 5: Label - 10(a): 9(ptr) Variable Function - 14(b): 9(ptr) Variable Function - 17(c): 9(ptr) Variable Function - 23(d): 9(ptr) Variable Function - 25(e): 9(ptr) Variable Function - 13: 6(ptr) Bitcast 12 - Store 10(a) 13 - 16: 6(ptr) Bitcast 15 - Store 14(b) 16 - 20: 6(ptr) Load 10(a) - 21: 6(ptr) Load 14(b) - 22: 6(ptr) Select 19 20 21 - Store 17(c) 22 - 24: 6(ptr) Load 14(b) - Store 23(d) 24 - 27: 6(ptr) Bitcast 26 - 29: 6(ptr) Bitcast 28 - 30: 6(ptr) Select 19 27 29 - Store 25(e) 30 + 11(a): 10(ptr) Variable Function + 15(b): 10(ptr) Variable Function + 18(c): 10(ptr) Variable Function + 24(d): 10(ptr) Variable Function + 26(e): 10(ptr) Variable Function + 29(f): 10(ptr) Variable Function + 14: 6(ptr) Bitcast 13 + Store 11(a) 14 + 17: 6(ptr) Bitcast 16 + Store 15(b) 17 + 21: 6(ptr) Load 11(a) + 22: 6(ptr) Load 15(b) + 23: 6(ptr) Select 20 21 22 + Store 18(c) 23 + 25: 6(ptr) Load 15(b) + Store 24(d) 25 + 28: 6(ptr) Bitcast 27 + Store 26(e) 28 + 30: 6(ptr) Load 11(a) + 33: 32(ptr) AccessChain 30 31 + 34: 8(int) Load 33 Aligned 4 + 36: 19(bool) INotEqual 34 35 + 38: 6(ptr) Bitcast 37 + 40: 6(ptr) Bitcast 39 + 41: 6(ptr) Select 36 38 40 + Store 29(f) 41 Return FunctionEnd diff --git a/Test/spv.bufferhandle16.frag b/Test/spv.bufferhandle16.frag index 02e75f5b..168148cc 100644 --- a/Test/spv.bufferhandle16.frag +++ b/Test/spv.bufferhandle16.frag @@ -6,6 +6,7 @@ layout(buffer_reference) buffer T1 { int x; + bool y; }; layout(buffer_reference) buffer T2 { int x; @@ -13,6 +14,7 @@ layout(buffer_reference) buffer T2 { const int s = int(uint64_t(T1(T2(uint64_t(3))))); int x[s]; +const uint64_t t = uint64_t(true ? T2(uint64_t(10)) : T2(uint64_t(11))); void main() { @@ -20,4 +22,5 @@ void main() T1 c = true ? a : b; T1 d = (a,b); T1 e = true ? T1(uint64_t(6)) : T1(uint64_t(7)); + T1 f = a.y ? T1(uint64_t(8)) : T1(uint64_t(9)); } diff --git a/glslang/MachineIndependent/Intermediate.cpp b/glslang/MachineIndependent/Intermediate.cpp index 5f1380db..5e9c7848 100644 --- a/glslang/MachineIndependent/Intermediate.cpp +++ b/glslang/MachineIndependent/Intermediate.cpp @@ -725,19 +725,6 @@ TIntermTyped* TIntermediate::createConversion(TBasicType convertTo, TIntermTyped return newNode; } -// Convert a constant that is a reference type to a uint64_t constant plus a -// constructor instruction. This is needed because SPIR-V doesn't support -// OpConstant on pointer types. -TIntermTyped* TIntermediate::addConstantReferenceConversion(TIntermTyped* node) -{ - if (node->getType().getBasicType() == EbtReference && node->getAsConstantUnion()) { - const TType &type = node->getType(); - node = addBuiltInFunctionCall(node->getLoc(), EOpConvPtrToUint64, true, node, TType(EbtUint64)); - node = addUnaryNode(EOpConstructReference, node, node->getLoc(), type); - } - return node; -} - TIntermTyped* TIntermediate::addConversion(TBasicType convertTo, TIntermTyped* node) const { return createConversion(convertTo, node); @@ -775,9 +762,6 @@ TIntermediate::addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* no return std::make_tuple(node0, node1); } - node0 = addConstantReferenceConversion(node0); - node1 = addConstantReferenceConversion(node1); - auto promoteTo = std::make_tuple(EbtNumTypes, EbtNumTypes); switch (op) { @@ -897,8 +881,6 @@ TIntermTyped* TIntermediate::addConversion(TOperator op, const TType& type, TInt if (!isConversionAllowed(op, node)) return nullptr; - node = addConstantReferenceConversion(node); - // Otherwise, if types are identical, no problem if (type == node->getType()) return node; diff --git a/glslang/MachineIndependent/ParseHelper.cpp b/glslang/MachineIndependent/ParseHelper.cpp index 7bd3c5f5..e1e2ee91 100644 --- a/glslang/MachineIndependent/ParseHelper.cpp +++ b/glslang/MachineIndependent/ParseHelper.cpp @@ -6527,11 +6527,7 @@ TIntermNode* TParseContext::executeInitializer(const TSourceLoc& loc, TIntermTyp // We either have a folded constant in getAsConstantUnion, or we have to use // the initializer's subtree in the AST to represent the computation of a // specialization constant. - // A third case arises when a reference type is made non-constant due to - // addConstantReferenceConversion, but reference types can't be const, so - // this is an error. - assert(initializer->getAsConstantUnion() || initializer->getType().getQualifier().isSpecConstant() || - initializer->getType().getBasicType() == EbtReference); + assert(initializer->getAsConstantUnion() || initializer->getType().getQualifier().isSpecConstant()); if (initializer->getAsConstantUnion()) variable->setConstArray(initializer->getAsConstantUnion()->getConstArray()); else { diff --git a/glslang/MachineIndependent/localintermediate.h b/glslang/MachineIndependent/localintermediate.h index 4254fb45..ba177258 100644 --- a/glslang/MachineIndependent/localintermediate.h +++ b/glslang/MachineIndependent/localintermediate.h @@ -501,7 +501,6 @@ public: TIntermTyped* addConversion(TBasicType convertTo, TIntermTyped* node) const; void addBiShapeConversion(TOperator, TIntermTyped*& lhsNode, TIntermTyped*& rhsNode); TIntermTyped* addShapeConversion(const TType&, TIntermTyped*); - TIntermTyped* addConstantReferenceConversion(TIntermTyped* node); TIntermTyped* addBinaryMath(TOperator, TIntermTyped* left, TIntermTyped* right, TSourceLoc); TIntermTyped* addAssign(TOperator op, TIntermTyped* left, TIntermTyped* right, TSourceLoc); TIntermTyped* addIndex(TOperator op, TIntermTyped* base, TIntermTyped* index, TSourceLoc);