Merge pull request #1874 from KhronosGroup/inherit-memory-qualifiers

Test/310.inheritMemory.frag

+#version 310 es
+precision mediump float;
+
+struct S {
+    float buff[10];
+};
+
+layout(std430, binding=2) readonly buffer RoBuff {
+    float buff_ro[10];
+    S s_ro;
+} ro_buffer;
+
+layout(std430, binding=2) buffer Buff {
+    float buff[10];
+    S s;
+} non_ro_buffer;
+
+void non_ro_fun(float[10] buff) { }
+void non_ro_funf(float el) { }
+void non_ro_funS(S s) { }
+
+out vec4 fragColor;
+
+void main()
+{
+    S s;
+
+    non_ro_fun(s.buff);
+    non_ro_funf(s.buff[3]);
+    non_ro_funS(s);
+
+    non_ro_fun(non_ro_buffer.buff);
+    non_ro_fun(non_ro_buffer.s.buff);
+    non_ro_funf(non_ro_buffer.buff[3]);
+    non_ro_funf(non_ro_buffer.s.buff[3]);
+    non_ro_funS(non_ro_buffer.s);
+
+    non_ro_fun(ro_buffer.buff_ro);
+    non_ro_fun(ro_buffer.s_ro.buff);
+    non_ro_funf(ro_buffer.buff_ro[3]);
+    non_ro_funf(ro_buffer.s_ro.buff[3]);
+    non_ro_funS(ro_buffer.s_ro);
+}

Test/baseResults/430.comp.out

@@ -55,9 +55,9 @@ ERROR: node is still EOpNull!
 0:63    Function Parameters: 
 0:65    Sequence
 0:65      move second child to first child ( temp float)
-0:65        direct index (layout( column_major shared) temp float)
-0:65          values: direct index for structure (layout( column_major shared) buffer unsized 3-element array of float)
-0:65            'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) buffer int value, layout( column_major shared) buffer unsized 3-element array of float values})
+0:65        direct index (layout( column_major shared) readonly temp float)
+0:65          values: direct index for structure (layout( column_major shared) readonly buffer unsized 3-element array of float)
+0:65            'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) readonly buffer int value, layout( column_major shared) readonly buffer unsized 3-element array of float values})
 0:65            Constant:
 0:65              1 (const int)
 0:65          Constant:
@@ -65,8 +65,8 @@ ERROR: node is still EOpNull!
 0:65        Constant:
 0:65          4.700000
 0:66      array length ( temp int)
-0:66        values: direct index for structure (layout( column_major shared) buffer unsized 3-element array of float)
-0:66          'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) buffer int value, layout( column_major shared) buffer unsized 3-element array of float values})
+0:66        values: direct index for structure (layout( column_major shared) readonly buffer unsized 3-element array of float)
+0:66          'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) readonly buffer int value, layout( column_major shared) readonly buffer unsized 3-element array of float values})
 0:66          Constant:
 0:66            1 (const int)
 0:67      Barrier ( global void)
@@ -140,7 +140,7 @@ ERROR: node is still EOpNull!
 0:?     'arrX' ( global 2-element array of int)
 0:?     'arrY' ( global 1-element array of int)
 0:?     'arrZ' ( global 4096-element array of int)
-0:?     'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) buffer int value, layout( column_major shared) buffer unsized 3-element array of float values})
+0:?     'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) readonly buffer int value, layout( column_major shared) readonly buffer unsized 3-element array of float values})
 0:?     'roll' ( uniform double)
 0:?     'destTex' ( writeonly uniform image2D)
 0:?     'inbi' ( in block{ in int a})
@@ -201,7 +201,7 @@ ERROR: node is still EOpNull!
 0:?     'arrX' ( global 2-element array of int)
 0:?     'arrY' ( global 1-element array of int)
 0:?     'arrZ' ( global 4096-element array of int)
-0:?     'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) buffer int value, layout( column_major shared) buffer unsized 3-element array of float values})
+0:?     'ro' (layout( column_major shared) readonly buffer block{layout( column_major shared) readonly buffer int value, layout( column_major shared) readonly buffer unsized 3-element array of float values})
 0:?     'roll' ( uniform double)
 0:?     'destTex' ( writeonly uniform image2D)
 0:?     'inbi' ( in block{ in int a})

Test/baseResults/specExamples.vert.out

@@ -307,7 +307,7 @@ ERROR: node is still EOpNull!
 0:?     'c' ( in 4-component vector of float)
 0:?     'd' ( in 4-component vector of float)
 0:?     'v' ( noContraction smooth out 4-component vector of float)
-0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) readonly buffer 4-component vector of float member1, layout( column_major shared) buffer 4-component vector of float member2})
+0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1, layout( column_major shared) coherent buffer 4-component vector of float member2})
 0:?     'anon@7' (layout( column_major shared) buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1A, layout( column_major shared) coherent buffer 4-component vector of float member2A})
 0:?     'shv' ( shared 4-component vector of float)
 0:?     'img1' (layout( rgba32f) uniform image2D)
@@ -590,7 +590,7 @@ ERROR: node is still EOpNull!
 0:?     'c' ( in 4-component vector of float)
 0:?     'd' ( in 4-component vector of float)
 0:?     'v' ( noContraction smooth out 4-component vector of float)
-0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) readonly buffer 4-component vector of float member1, layout( column_major shared) buffer 4-component vector of float member2})
+0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1, layout( column_major shared) coherent buffer 4-component vector of float member2})
 0:?     'anon@7' (layout( column_major shared) buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1A, layout( column_major shared) coherent buffer 4-component vector of float member2A})
 0:?     'shv' ( shared 4-component vector of float)
 0:?     'img1' (layout( rgba32f) uniform image2D)

Test/baseResults/specExamplesConf.vert.out

@@ -308,7 +308,7 @@ ERROR: node is still EOpNull!
 0:?     'c' ( in 4-component vector of float)
 0:?     'd' ( in 4-component vector of float)
 0:?     'v' ( noContraction smooth out 4-component vector of float)
-0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) readonly buffer 4-component vector of float member1, layout( column_major shared) buffer 4-component vector of float member2})
+0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1, layout( column_major shared) coherent buffer 4-component vector of float member2})
 0:?     'anon@7' (layout( column_major shared) buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1A, layout( column_major shared) coherent buffer 4-component vector of float member2A})
 0:?     'shv' ( shared 4-component vector of float)
 0:?     'img1' (layout( rgba32f) uniform image2D)
@@ -591,7 +591,7 @@ ERROR: node is still EOpNull!
 0:?     'c' ( in 4-component vector of float)
 0:?     'd' ( in 4-component vector of float)
 0:?     'v' ( noContraction smooth out 4-component vector of float)
-0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) readonly buffer 4-component vector of float member1, layout( column_major shared) buffer 4-component vector of float member2})
+0:?     'anon@6' (layout( column_major shared) coherent buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1, layout( column_major shared) coherent buffer 4-component vector of float member2})
 0:?     'anon@7' (layout( column_major shared) buffer block{layout( column_major shared) coherent readonly buffer 4-component vector of float member1A, layout( column_major shared) coherent buffer 4-component vector of float member2A})
 0:?     'shv' ( shared 4-component vector of float)
 0:?     'img1' (layout( rgba32f) uniform image2D)

Test/baseResults/spv.coopmat.comp.out

@@ -282,7 +282,7 @@ spv.coopmat.comp
              101:    100(ptr) AccessChain 91(block16) 99
              102:     85(ptr) Load 101 MakePointerVisibleKHR NonPrivatePointerKHR 71
              104:    103(ptr) AccessChain 102 58 31
-             105:          32 CooperativeMatrixLoadNV 104 74 76 Aligned 16
+             105:          32 CooperativeMatrixLoadNV 104 74 76 Aligned MakePointerVisibleKHR NonPrivatePointerKHR 16 71
                               Store 98(tempArg) 105
              106:          32 Load 98(tempArg)
                               Store 34(m) 106
@@ -290,7 +290,7 @@ spv.coopmat.comp
              108:    100(ptr) AccessChain 91(block16) 99
              109:     85(ptr) Load 108 MakePointerVisibleKHR NonPrivatePointerKHR 71
              110:    103(ptr) AccessChain 109 58 31
-                              CooperativeMatrixStoreNV 110 107 74 76 Aligned 16
+                              CooperativeMatrixStoreNV 110 107 74 76 Aligned MakePointerAvailableKHR NonPrivatePointerKHR 16 71
              113:          50 Load 112(A)
              115:          10 Load 114(B)
              117:          32 Load 116(C)

glslang/Include/Types.h

@@ -539,20 +539,6 @@ public:
         writeonly    = false;
     }
 
-    // Drop just the storage qualification, which perhaps should
-    // never be done, as it is fundamentally inconsistent, but need to
-    // explore what downstream consumers need.
-    // E.g., in a dereference, it is an inconsistency between:
-    // A) partially dereferenced resource is still in the storage class it started in
-    // B) partially dereferenced resource is a new temporary object
-    // If A, then nothing should change, if B, then everything should change, but this is half way.
-    void makePartialTemporary()
-    {
-        storage      = EvqTemporary;
-        specConstant = false;
-        nonUniform   = false;
-    }
-
     const char*         semanticName;
     TStorageQualifier   storage   : 6;
     TBuiltInVariable    builtIn   : 9;

glslang/MachineIndependent/ParseHelper.cpp

@@ -473,7 +473,7 @@ TIntermTyped* TParseContext::handleBracketDereference(const TSourceLoc& loc, TIn
                 profileRequires(base->getLoc(), EEsProfile, 320, Num_AEP_gpu_shader5, AEP_gpu_shader5,
                                 "variable indexing uniform block array");
             else {
-                // input/output blocks either don't exist or can be variable indexed
+                // input/output blocks either don't exist or can't be variably indexed
             }
         } else if (language == EShLangFragment && base->getQualifier().isPipeOutput())
             requireProfile(base->getLoc(), ~EEsProfile, "variable indexing fragment shader output array");
@@ -487,8 +487,8 @@ TIntermTyped* TParseContext::handleBracketDereference(const TSourceLoc& loc, TIn
         result = intermediate.addIndex(EOpIndexIndirect, base, index, loc);
     }
 
-    // Insert valid dereferenced result
-    TType newType(base->getType(), 0);  // dereferenced type
+    // Insert valid dereferenced result type
+    TType newType(base->getType(), 0);
     if (base->getType().getQualifier().isConstant() && index->getQualifier().isConstant()) {
         newType.getQualifier().storage = EvqConst;
         // If base or index is a specialization constant, the result should also be a specialization constant.
@@ -496,11 +496,14 @@ TIntermTyped* TParseContext::handleBracketDereference(const TSourceLoc& loc, TIn
             newType.getQualifier().makeSpecConstant();
         }
     } else {
-        newType.getQualifier().makePartialTemporary();
+        newType.getQualifier().storage = EvqTemporary;
+        newType.getQualifier().specConstant = false;
     }
     result->setType(newType);
 
 #ifndef GLSLANG_WEB
+    inheritMemoryQualifiers(base->getQualifier(), result->getWritableType().getQualifier());
+
     // Propagate nonuniform
     if (base->getQualifier().isNonUniform() || index->getQualifier().isNonUniform())
         result->getWritableType().getQualifier().nonUniform = true;
@@ -881,6 +884,7 @@ TIntermTyped* TParseContext::handleDotDereference(const TSourceLoc& loc, TInterm
                 if ((*fields)[member].type->getQualifier().isIo())
                     intermediate.addIoAccessed(field);
             }
+            inheritMemoryQualifiers(base->getQualifier(), result->getWritableType().getQualifier());
         } else
             error(loc, "no such field in structure", field.c_str(), "");
     } else
@@ -7124,6 +7128,23 @@ TIntermTyped* TParseContext::constructAggregate(TIntermNode* node, const TType&
     return converted;
 }
 
+// If a memory qualifier is present in 'to', also make it present in 'from'.
+void TParseContext::inheritMemoryQualifiers(const TQualifier& from, TQualifier& to)
+{
+#ifndef GLSLANG_WEB
+    if (from.isReadOnly())
+        to.readonly = from.readonly;
+    if (from.isWriteOnly())
+        to.writeonly = from.writeonly;
+    if (from.coherent)
+        to.coherent = from.coherent;
+    if (from.volatil)
+        to.volatil = from.volatil;
+    if (from.restrict)
+        to.restrict = from.restrict;
+#endif
+}
+
 //
 // Do everything needed to add an interface block.
 //
@@ -7139,7 +7160,7 @@ void TParseContext::declareBlock(const TSourceLoc& loc, TTypeList& typeList, con
             requireProfile(loc, ~EEsProfile, "array-of-array of block");
     }
 
-    // fix and check for member storage qualifiers and types that don't belong within a block
+    // Inherit and check member storage qualifiers WRT to the block-level qualifier.
     for (unsigned int member = 0; member < typeList.size(); ++member) {
         TType& memberType = *typeList[member].type;
         TQualifier& memberQualifier = memberType.getQualifier();
@@ -7149,6 +7170,7 @@ void TParseContext::declareBlock(const TSourceLoc& loc, TTypeList& typeList, con
             error(memberLoc, "member storage qualifier cannot contradict block storage qualifier", memberType.getFieldName().c_str(), "");
         memberQualifier.storage = currentBlockQualifier.storage;
 #ifndef GLSLANG_WEB
+        inheritMemoryQualifiers(currentBlockQualifier, memberQualifier);
         if (currentBlockQualifier.perPrimitiveNV)
             memberQualifier.perPrimitiveNV = currentBlockQualifier.perPrimitiveNV;
         if (currentBlockQualifier.perViewNV)

glslang/MachineIndependent/ParseHelper.h

@@ -410,6 +410,7 @@ public:
     TIntermTyped* addConstructor(const TSourceLoc&, TIntermNode*, const TType&);
     TIntermTyped* constructAggregate(TIntermNode*, const TType&, int, const TSourceLoc&);
     TIntermTyped* constructBuiltIn(const TType&, TOperator, TIntermTyped*, const TSourceLoc&, bool subset);
+    void inheritMemoryQualifiers(const TQualifier& from, TQualifier& to);
     void declareBlock(const TSourceLoc&, TTypeList& typeList, const TString* instanceName = 0, TArraySizes* arraySizes = 0);
     void blockStageIoCheck(const TSourceLoc&, const TQualifier&);
     void blockQualifierCheck(const TSourceLoc&, const TQualifier&, bool instanceName);

gtests/AST.FromFile.cpp

@@ -121,6 +121,7 @@ INSTANTIATE_TEST_CASE_P(
         "310.tesc",
         "310.tese",
         "310implicitSizeArrayError.vert",
+        "310.inheritMemory.frag",
         "310AofA.vert",
         "310runtimeArray.vert",
         "320.comp",