Merge pull request #1699 from jeffbolznv/buffer_reference_fold

Handle buffer references vs 'const'

Test/baseResults/spv.bufferhandle16.frag.out

@@ -0,0 +1,76 @@
+spv.bufferhandle16.frag
+// Module Version 10000
+// Generated by (magic number): 80007
+// Id's are bound by 37
+
+                              Capability Shader
+                              Capability Int64
+                              Capability CapabilityPhysicalStorageBufferAddressesEXT
+                              Extension  "SPV_EXT_physical_storage_buffer"
+               1:             ExtInstImport  "GLSL.std.450"
+                              MemoryModel PhysicalStorageBuffer64EXT GLSL450
+                              EntryPoint Fragment 4  "main"
+                              ExecutionMode 4 OriginUpperLeft
+                              Source GLSL 450
+                              SourceExtension  "GL_EXT_buffer_reference"
+                              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
+               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
+         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
+                              Return
+                              FunctionEnd

Test/baseResults/spv.bufferhandle17_Errors.frag.out

@@ -0,0 +1,9 @@
+spv.bufferhandle17_Errors.frag
+ERROR: 0:11: 'qualifier' : variables with reference type can't have qualifier 'const' 
+ERROR: 0:16: 'qualifier' : variables with reference type can't have qualifier 'const' 
+ERROR: 0:18: '==' : can't use with reference types 
+ERROR: 0:19: '!=' : can't use with reference types 
+ERROR: 4 compilation errors.  No code generated.
+
+
+SPIR-V is not generated for failed compile or link

Test/spv.bufferhandle16.frag

@@ -0,0 +1,23 @@
+#version 450
+
+#extension GL_EXT_shader_explicit_arithmetic_types_int64 : enable
+#extension GL_EXT_buffer_reference : enable
+#extension GL_EXT_scalar_block_layout : enable
+
+layout(buffer_reference) buffer T1 {
+    int x;
+};
+layout(buffer_reference) buffer T2 {
+    int x;
+};
+
+const int s = int(uint64_t(T1(T2(uint64_t(3)))));
+int x[s];
+
+void main()
+{
+    T1 a = T1(uint64_t(4)), b = T1(uint64_t(5));
+    T1 c = true ? a : b;
+    T1 d = (a,b);
+    T1 e = true ? T1(uint64_t(6)) : T1(uint64_t(7));
+}

Test/spv.bufferhandle17_Errors.frag

@@ -0,0 +1,20 @@
+#version 450
+
+#extension GL_EXT_shader_explicit_arithmetic_types_int64 : enable
+#extension GL_EXT_buffer_reference : enable
+#extension GL_EXT_scalar_block_layout : enable
+
+layout(buffer_reference) buffer T1 {
+    int x;
+};
+
+const T1 a = T1(uint64_t(2));
+
+void main()
+{
+    T1 b, c;
+    const T1 d = b;
+
+    b == c;
+    b != c;
+}

glslang/Include/Types.h

@@ -1628,6 +1628,7 @@ public:
             case EbtInt64:
             case EbtUint64:
             case EbtBool:
+            case EbtReference:
                 return true;
             default:
                 return false;

glslang/MachineIndependent/Constant.cpp

@@ -941,6 +941,10 @@ TIntermTyped* TIntermConstantUnion::fold(TOperator op, const TType& returnType)
             newConstArray[i].setDConst(unionArray[i].getDConst()); break;
         case EOpConvDoubleToFloat:
             newConstArray[i].setDConst(unionArray[i].getDConst()); break;
+        case EOpConvPtrToUint64:
+        case EOpConvUint64ToPtr:
+        case EOpConstructReference:
+            newConstArray[i].setU64Const(unionArray[i].getU64Const()); break;
 
 
 

glslang/MachineIndependent/Intermediate.cpp

@@ -725,6 +725,19 @@ 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);
@@ -743,7 +756,7 @@ TIntermTyped* TIntermediate::addConversion(TBasicType convertTo, TIntermTyped* n
 // Returns the converted pair of nodes.
 // Returns <nullptr, nullptr> when there is no conversion.
 std::tuple<TIntermTyped*, TIntermTyped*>
-TIntermediate::addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1) const
+TIntermediate::addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1)
 {
     if (!isConversionAllowed(op, node0) || !isConversionAllowed(op, node1))
         return std::make_tuple(nullptr, nullptr);
@@ -762,6 +775,9 @@ 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) {
@@ -876,11 +892,13 @@ TIntermediate::addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* no
 //
 // Return nullptr if a conversion can't be done.
 //
-TIntermTyped* TIntermediate::addConversion(TOperator op, const TType& type, TIntermTyped* node) const
+TIntermTyped* TIntermediate::addConversion(TOperator op, const TType& type, TIntermTyped* node)
 {
     if (!isConversionAllowed(op, node))
         return nullptr;
 
+    node = addConstantReferenceConversion(node);
+
     // Otherwise, if types are identical, no problem
     if (type == node->getType())
         return node;

glslang/MachineIndependent/ParseHelper.cpp

@@ -4435,6 +4435,12 @@ void TParseContext::opaqueCheck(const TSourceLoc& loc, const TType& type, const
         error(loc, "can't use with samplers or structs containing samplers", op, "");
 }
 
+void TParseContext::referenceCheck(const TSourceLoc& loc, const TType& type, const char* op)
+{
+    if (containsFieldWithBasicType(type, EbtReference))
+        error(loc, "can't use with reference types", op, "");
+}
+
 void TParseContext::storage16BitAssignmentCheck(const TSourceLoc& loc, const TType& type, const char* op)
 {
     if (type.getBasicType() == EbtStruct && containsFieldWithBasicType(type, EbtFloat16))
@@ -6290,6 +6296,9 @@ TIntermNode* TParseContext::declareVariable(const TSourceLoc& loc, TString& iden
 #ifdef NV_EXTENSIONS
     accStructNVCheck(loc, type, identifier);
 #endif
+    if (type.getQualifier().storage == EvqConst && type.containsBasicType(EbtReference)) {
+        error(loc, "variables with reference type can't have qualifier 'const'", "qualifier", "");
+    }
 
     if (type.getQualifier().storage != EvqUniform && type.getQualifier().storage != EvqBuffer) {
         if (type.containsBasicType(EbtFloat16))
@@ -6518,7 +6527,11 @@ 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.
-        assert(initializer->getAsConstantUnion() || initializer->getType().getQualifier().isSpecConstant());
+        // 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);
         if (initializer->getAsConstantUnion())
             variable->setConstArray(initializer->getAsConstantUnion()->getConstArray());
         else {
@@ -6860,7 +6873,7 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T
 
     case EOpConstructUint64:
         if (type.isScalar() && node->getType().getBasicType() == EbtReference) {
-            TIntermUnary* newNode = intermediate.addUnaryNode(EOpConvPtrToUint64, node, node->getLoc(), type);
+            TIntermTyped* newNode = intermediate.addBuiltInFunctionCall(node->getLoc(), EOpConvPtrToUint64, true, node, type);
             return newNode;
         }
         // fall through
@@ -6885,11 +6898,11 @@ TIntermTyped* TParseContext::constructBuiltIn(const TType& type, TOperator op, T
     case EOpConstructReference:
         // construct reference from reference
         if (node->getType().getBasicType() == EbtReference) {
-            newNode = intermediate.addUnaryNode(EOpConstructReference, node, node->getLoc(), type);
+            newNode = intermediate.addBuiltInFunctionCall(node->getLoc(), EOpConstructReference, true, node, type);
             return newNode;
         // construct reference from uint64
         } else if (node->getType().isScalar() && node->getType().getBasicType() == EbtUint64) {
-            TIntermUnary* newNode = intermediate.addUnaryNode(EOpConvUint64ToPtr, node, node->getLoc(), type);
+            TIntermTyped* newNode = intermediate.addBuiltInFunctionCall(node->getLoc(), EOpConvUint64ToPtr, true, node, type);
             return newNode;
         } else {
             return nullptr;

glslang/MachineIndependent/ParseHelper.h

@@ -370,6 +370,7 @@ public:
     void nestedStructCheck(const TSourceLoc&);
     void arrayObjectCheck(const TSourceLoc&, const TType&, const char* op);
     void opaqueCheck(const TSourceLoc&, const TType&, const char* op);
+    void referenceCheck(const TSourceLoc&, const TType&, const char* op);
     void storage16BitAssignmentCheck(const TSourceLoc&, const TType&, const char* op);
     void specializationCheck(const TSourceLoc&, const TType&, const char* op);
     void structTypeCheck(const TSourceLoc&, TPublicType&);

glslang/MachineIndependent/glslang.y

@@ -613,6 +613,7 @@ equality_expression
         parseContext.arrayObjectCheck($2.loc, $1->getType(), "array comparison");
         parseContext.opaqueCheck($2.loc, $1->getType(), "==");
         parseContext.specializationCheck($2.loc, $1->getType(), "==");
+        parseContext.referenceCheck($2.loc, $1->getType(), "==");
         $$ = parseContext.handleBinaryMath($2.loc, "==", EOpEqual, $1, $3);
         if ($$ == 0)
             $$ = parseContext.intermediate.addConstantUnion(false, $2.loc);
@@ -621,6 +622,7 @@ equality_expression
         parseContext.arrayObjectCheck($2.loc, $1->getType(), "array comparison");
         parseContext.opaqueCheck($2.loc, $1->getType(), "!=");
         parseContext.specializationCheck($2.loc, $1->getType(), "!=");
+        parseContext.referenceCheck($2.loc, $1->getType(), "!=");
         $$ = parseContext.handleBinaryMath($2.loc, "!=", EOpNotEqual, $1, $3);
         if ($$ == 0)
             $$ = parseContext.intermediate.addConstantUnion(false, $2.loc);

glslang/MachineIndependent/localintermediate.h

@@ -495,12 +495,13 @@ public:
     TIntermSymbol* addSymbol(const TVariable&, const TSourceLoc&);
     TIntermSymbol* addSymbol(const TType&, const TSourceLoc&);
     TIntermSymbol* addSymbol(const TIntermSymbol&);
-    TIntermTyped* addConversion(TOperator, const TType&, TIntermTyped*) const;
+    TIntermTyped* addConversion(TOperator, const TType&, TIntermTyped*);
-    std::tuple<TIntermTyped*, TIntermTyped*> addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1) const;
+    std::tuple<TIntermTyped*, TIntermTyped*> addConversion(TOperator op, TIntermTyped* node0, TIntermTyped* node1);
     TIntermTyped* addUniShapeConversion(TOperator, const TType&, TIntermTyped*);
     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);

gtests/Spv.FromFile.cpp

@@ -272,6 +272,8 @@ INSTANTIATE_TEST_CASE_P(
         "spv.bufferhandle13.frag",
         "spv.bufferhandle14.frag",
         "spv.bufferhandle15.frag",
+        "spv.bufferhandle16.frag",
+        "spv.bufferhandle17_Errors.frag",
         "spv.bufferhandle2.frag",
         "spv.bufferhandle3.frag",
         "spv.bufferhandle4.frag",