Translator: Memory qualifiers on SSBO fields

src/compiler/translator/OutputGLSLBase.cpp

@@ -212,7 +212,8 @@ std::string TOutputGLSLBase::getCommonLayoutQualifiers(TIntermTyped *variable)
     return out.str();
 }
 
-// Outputs what comes after in/out/uniform/buffer storage qualifier.
+// Outputs memory qualifiers applied to images, buffers and its fields, as well as image function
+// arguments.
 std::string TOutputGLSLBase::getMemoryQualifiers(const TType &type)
 {
     std::ostringstream out;
@@ -220,31 +221,26 @@ std::string TOutputGLSLBase::getMemoryQualifiers(const TType &type)
     const TMemoryQualifier &memoryQualifier = type.getMemoryQualifier();
     if (memoryQualifier.readonly)
     {
-        ASSERT(IsImage(type.getBasicType()) || IsStorageBuffer(type.getQualifier()));
         out << "readonly ";
     }
 
     if (memoryQualifier.writeonly)
     {
-        ASSERT(IsImage(type.getBasicType()) || IsStorageBuffer(type.getQualifier()));
         out << "writeonly ";
     }
 
     if (memoryQualifier.coherent)
     {
-        ASSERT(IsImage(type.getBasicType()) || IsStorageBuffer(type.getQualifier()));
         out << "coherent ";
     }
 
     if (memoryQualifier.restrictQualifier)
     {
-        ASSERT(IsImage(type.getBasicType()) || IsStorageBuffer(type.getQualifier()));
         out << "restrict ";
     }
 
     if (memoryQualifier.volatileQualifier)
     {
-        ASSERT(IsImage(type.getBasicType()) || IsStorageBuffer(type.getQualifier()));
         out << "volatile ";
     }
 
@@ -375,7 +371,9 @@ const char *TOutputGLSLBase::mapQualifierToString(TQualifier qualifier)
     return sh::getQualifierString(qualifier);
 }
 
-void TOutputGLSLBase::writeVariableType(const TType &type, const TSymbol *symbol)
+void TOutputGLSLBase::writeVariableType(const TType &type,
+                                        const TSymbol *symbol,
+                                        bool isFunctionArgument)
 {
     TQualifier qualifier = type.getQualifier();
     TInfoSinkBase &out   = objSink();
@@ -391,6 +389,12 @@ void TOutputGLSLBase::writeVariableType(const TType &type, const TSymbol *symbol
     {
         writeQualifier(qualifier, type, symbol);
     }
+    if (isFunctionArgument)
+    {
+        // Function arguments are the only place (other than image/SSBO/field declaration) where
+        // memory qualifiers can appear.
+        out << getMemoryQualifiers(type);
+    }
 
     // Declare the struct if we have not done so already.
     if (type.getBasicType() == EbtStruct && !structDeclared(type.getStruct()))
@@ -420,7 +424,7 @@ void TOutputGLSLBase::writeFunctionParameters(const TFunction *func)
     {
         const TVariable *param = func->getParam(i);
         const TType &type      = param->getType();
-        writeVariableType(type, param);
+        writeVariableType(type, param, true);
 
         if (param->symbolType() != SymbolType::Empty)
             out << " " << hashName(param);
@@ -977,7 +981,7 @@ void TOutputGLSLBase::visitFunctionPrototype(TIntermFunctionPrototype *node)
     TInfoSinkBase &out = objSink();
 
     const TType &type = node->getType();
-    writeVariableType(type, node->getFunction());
+    writeVariableType(type, node->getFunction(), false);
     if (type.isArray())
         out << ArrayString(type);
 
@@ -1088,7 +1092,8 @@ bool TOutputGLSLBase::visitDeclaration(Visit visit, TIntermDeclaration *node)
         TIntermTyped *variable          = sequence.front()->getAsTyped();
         writeLayoutQualifier(variable);
         TIntermSymbol *symbolNode = variable->getAsSymbolNode();
-        writeVariableType(variable->getType(), symbolNode ? &symbolNode->variable() : nullptr);
+        writeVariableType(variable->getType(), symbolNode ? &symbolNode->variable() : nullptr,
+                          false);
         if (variable->getAsSymbolNode() == nullptr ||
             variable->getAsSymbolNode()->variable().symbolType() != SymbolType::Empty)
         {
@@ -1361,6 +1366,7 @@ void TOutputGLSLBase::declareInterfaceBlock(const TInterfaceBlock *interfaceBloc
     for (const TField *field : fields)
     {
         writeFieldLayoutQualifier(field);
+        out << getMemoryQualifiers(*field->type());
 
         if (writeVariablePrecision(field->type()->getPrecision()))
             out << " ";

src/compiler/translator/OutputGLSLBase.h

@@ -46,7 +46,9 @@ class TOutputGLSLBase : public TIntermTraverser
     virtual void writeFieldLayoutQualifier(const TField *field);
     void writeInvariantQualifier(const TType &type);
     void writePreciseQualifier(const TType &type);
-    virtual void writeVariableType(const TType &type, const TSymbol *symbol);
+    virtual void writeVariableType(const TType &type,
+                                   const TSymbol *symbol,
+                                   bool isFunctionArgument);
     virtual bool writeVariablePrecision(TPrecision precision) = 0;
     void writeFunctionParameters(const TFunction *func);
     const TConstantUnion *writeConstantUnion(const TType &type, const TConstantUnion *pConstUnion);

src/compiler/translator/OutputVulkanGLSL.cpp

@@ -180,7 +180,9 @@ void TOutputVulkanGLSL::writeQualifier(TQualifier qualifier,
         << ") @@ ";
 }
 
-void TOutputVulkanGLSL::writeVariableType(const TType &type, const TSymbol *symbol)
+void TOutputVulkanGLSL::writeVariableType(const TType &type,
+                                          const TSymbol *symbol,
+                                          bool isFunctionArgument)
 {
     TType overrideType(type);
 
@@ -190,7 +192,7 @@ void TOutputVulkanGLSL::writeVariableType(const TType &type, const TSymbol *symb
         overrideType.setBasicType(EbtSampler2D);
     }
 
-    TOutputGLSL::writeVariableType(overrideType, symbol);
+    TOutputGLSL::writeVariableType(overrideType, symbol, isFunctionArgument);
 }
 
 void TOutputVulkanGLSL::writeStructType(const TStructure *structure)

src/compiler/translator/OutputVulkanGLSL.h

@@ -33,7 +33,9 @@ class TOutputVulkanGLSL : public TOutputGLSL
     void writeLayoutQualifier(TIntermTyped *variable) override;
     void writeFieldLayoutQualifier(const TField *field) override;
     void writeQualifier(TQualifier qualifier, const TType &type, const TSymbol *symbol) override;
-    void writeVariableType(const TType &type, const TSymbol *symbol) override;
+    void writeVariableType(const TType &type,
+                           const TSymbol *symbol,
+                           bool isFunctionArgument) override;
 };
 
 }  // namespace sh

src/compiler/translator/OutputVulkanGLSLForMetal.h

@@ -31,7 +31,9 @@ class TOutputVulkanGLSLForMetal : public TOutputVulkanGLSL
   protected:
     bool visitGlobalQualifierDeclaration(Visit visit,
                                          TIntermGlobalQualifierDeclaration *node) override;
-    void writeVariableType(const TType &type, const TSymbol *symbol) override;
+    void writeVariableType(const TType &type,
+                           const TSymbol *symbol,
+                           bool isFunctionArgument) override;
 };
 
 }  // namespace sh

src/compiler/translator/OutputVulkanGLSLForMetal.mm

@@ -74,7 +74,9 @@ TOutputVulkanGLSLForMetal::TOutputVulkanGLSLForMetal(TInfoSinkBase &objSink,
                         compileOptions)
 {}
 
-void TOutputVulkanGLSLForMetal::writeVariableType(const TType &type, const TSymbol *symbol)
+void TOutputVulkanGLSLForMetal::writeVariableType(const TType &type,
+                                                  const TSymbol *symbol,
+                                                  bool isFunctionArgument)
 {
     TType overrideType(type);
 
@@ -84,7 +86,7 @@ void TOutputVulkanGLSLForMetal::writeVariableType(const TType &type, const TSymb
         overrideType.setInvariant(false);
     }
 
-    TOutputVulkanGLSL::writeVariableType(overrideType, symbol);
+    TOutputVulkanGLSL::writeVariableType(overrideType, symbol, isFunctionArgument);
 }
 
 bool TOutputVulkanGLSLForMetal::visitGlobalQualifierDeclaration(

src/tests/compiler_tests/BufferVariables_test.cpp

@@ -13,6 +13,7 @@
 #include "angle_gl.h"
 #include "gtest/gtest.h"
 #include "tests/test_utils/ShaderCompileTreeTest.h"
+#include "tests/test_utils/compiler_test.h"
 
 using namespace sh;
 
@@ -30,6 +31,15 @@ class BufferVariablesTest : public ShaderCompileTreeTest
     }
 };
 
+class BufferVariablesMatchTest : public MatchOutputCodeTest
+{
+  public:
+    BufferVariablesMatchTest() : MatchOutputCodeTest(GL_VERTEX_SHADER, 0, SH_ESSL_OUTPUT)
+    {
+        getResources()->MaxShaderStorageBufferBindings = 8;
+    }
+};
+
 // Test that the buffer qualifier described in GLSL ES 3.10 section 4.3.7 can be successfully
 // compiled.
 TEST_F(BufferVariablesTest, BasicShaderStorageBlockDeclaration)
@@ -642,3 +652,33 @@ TEST_F(BufferVariablesTest, RuntimeSizedVariableInNotLastInBuffer)
         FAIL() << "Shader compilation succeeded, expecting failure:\n" << mInfoLog;
     }
 }
+
+// Test that memory qualifiers are output.
+TEST_F(BufferVariablesMatchTest, MemoryQualifiers)
+{
+    const std::string &source =
+        R"(#version 310 es
+
+        layout(std430) coherent buffer buf
+        {
+            int defaultCoherent;
+            coherent ivec2 specifiedCoherent;
+            volatile ivec3 specifiedVolatile;
+            restrict ivec4 specifiedRestrict;
+            readonly float specifiedReadOnly;
+            writeonly vec2 specifiedWriteOnly;
+            volatile readonly vec3 specifiedMultiple;
+        };
+
+        void main()
+        {
+        })";
+    compile(source);
+    ASSERT_TRUE(foundInESSLCode("coherent highp int"));
+    ASSERT_TRUE(foundInESSLCode("coherent highp ivec2"));
+    ASSERT_TRUE(foundInESSLCode("coherent volatile highp ivec3"));
+    ASSERT_TRUE(foundInESSLCode("coherent restrict highp ivec4"));
+    ASSERT_TRUE(foundInESSLCode("readonly coherent highp float"));
+    ASSERT_TRUE(foundInESSLCode("writeonly coherent highp vec2"));
+    ASSERT_TRUE(foundInESSLCode("readonly coherent volatile highp vec3"));
+}