Improved fix for buffer reference constants
authorJeff Bolz <jbolz@nvidia.com>
Wed, 6 Mar 2019 05:27:09 +0000 (23:27 -0600)
committerJeff Bolz <jbolz@nvidia.com>
Wed, 6 Mar 2019 15:28:29 +0000 (09:28 -0600)
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
Test/baseResults/spv.bufferhandle16.frag.out
Test/spv.bufferhandle16.frag
glslang/MachineIndependent/Intermediate.cpp
glslang/MachineIndependent/ParseHelper.cpp
glslang/MachineIndependent/localintermediate.h

index 5b753ce19572e128d544fa414f7e7b9d07f02cc0..07f303509e192d1214101b9d39ef1ac3530f9986 100644 (file)
@@ -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;
index 79e5b6b60d92bc757289e75506705f0759f38f3f..a4cf44d1cb0198091dd0074a5847b14ec7876084 100644 (file)
@@ -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
index 02e75f5b61b00179e764e5ae147c7a960bbac2ae..168148ccb72784c11a4d48f3e020c1ee52146f22 100644 (file)
@@ -6,6 +6,7 @@
 \r
 layout(buffer_reference) buffer T1 {\r
     int x;\r
+    bool y;\r
 };\r
 layout(buffer_reference) buffer T2 {\r
     int x;\r
@@ -13,6 +14,7 @@ layout(buffer_reference) buffer T2 {
 \r
 const int s = int(uint64_t(T1(T2(uint64_t(3)))));\r
 int x[s];\r
+const uint64_t t = uint64_t(true ? T2(uint64_t(10)) : T2(uint64_t(11)));\r
 \r
 void main()\r
 {\r
@@ -20,4 +22,5 @@ void main()
     T1 c = true ? a : b;\r
     T1 d = (a,b);\r
     T1 e = true ? T1(uint64_t(6)) : T1(uint64_t(7));\r
+    T1 f = a.y ? T1(uint64_t(8)) : T1(uint64_t(9));\r
 }\r
index 5f1380db870c214840212189bdfbfd64d21b999d..5e9c784826d1506ad929d7f0b36e4ccb8af189c5 100644 (file)
@@ -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;
index 7bd3c5f5e64d57ce7c5ef50e057694555ba2f70e..e1e2ee91d89b924f7b8bef9ea506a526d357e773 100644 (file)
@@ -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 {
index 4254fb45b217f44e725d314d37c7a72902466462..ba1772580bda4a30b835095dd5a3e91a17c5e488 100644 (file)
@@ -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);