Vulkan: I/O blocks: Support unsized arrays

include/GLSLANG/ShaderVars.h

@@ -53,7 +53,7 @@ enum class BlockType
     BLOCK_BUFFER,
 
     // TODO: Remove gl_in from interface blocks and place it in varyings with the rest of shader I/O
-    // blocks, then remove GetInBlocks and getInBlocks everywhere.  http://anglebug.com/NNNN
+    // blocks, then remove GetInBlocks and getInBlocks everywhere.  http://anglebug.com/5423
     BLOCK_IN
 };
 

scripts/code_generation_hashes/ANGLE_shader_translator.json

@@ -6,11 +6,11 @@
   "src/compiler/translator/glslang.l":
     "a04170178bdc8511bff69ec4ff144576",
   "src/compiler/translator/glslang.y":
-    "d614cbc19923d104c98e3a5f8770b5be",
+    "1c7a2f4fa0f62ebc42f855bb8a8deeaf",
   "src/compiler/translator/glslang_lex_autogen.cpp":
     "5d6469e58f9855f93987b5ddbd663920",
   "src/compiler/translator/glslang_tab_autogen.cpp":
-    "a93ca4c95c635f4154e41586911d0182",
+    "821a6cccd98a7d89666bfb3fabee3585",
   "src/compiler/translator/glslang_tab_autogen.h":
     "840c12db9f7824afd05b5306c8816d95",
   "tools/flex-bison/linux/bison.sha1":

src/compiler/translator/BaseTypes.h

@@ -1097,8 +1097,14 @@ inline bool IsShaderIoBlock(TQualifier qualifier)
 {
     switch (qualifier)
     {
-        case EvqFragmentIn:
         case EvqVertexOut:
+        case EvqTessControlIn:
+        case EvqTessControlOut:
+        case EvqTessEvaluationIn:
+        case EvqTessEvaluationOut:
+        case EvqGeometryIn:
+        case EvqGeometryOut:
+        case EvqFragmentIn:
             return true;
         default:
             return false;

src/compiler/translator/DirectiveHandler.cpp

@@ -167,12 +167,22 @@ void TDirectiveHandler::handleExtension(const angle::pp::SourceLocation &loc,
         // OVR_multiview is implicitly enabled when OVR_multiview2 is enabled
         if (name == "GL_OVR_multiview2")
         {
-            const std::string multiview = "GL_OVR_multiview";
-            TExtensionBehavior::iterator iterMultiview =
-                mExtensionBehavior.find(GetExtensionByName(multiview.c_str()));
-            if (iterMultiview != mExtensionBehavior.end())
+            constexpr char kMultiviewExtName[] = "GL_OVR_multiview";
+            iter = mExtensionBehavior.find(GetExtensionByName(kMultiviewExtName));
+            if (iter != mExtensionBehavior.end())
             {
-                iterMultiview->second = behaviorVal;
+                iter->second = behaviorVal;
+            }
+        }
+        // EXT_shader_io_blocks is implicitly enabled when EXT_geometry_shader or
+        // EXT_tessellation_shader is enabled.
+        if (name == "GL_EXT_geometry_shader" || name == "GL_EXT_tessellation_shader")
+        {
+            constexpr char kIOBlocksExtName[] = "GL_EXT_shader_io_blocks";
+            iter = mExtensionBehavior.find(GetExtensionByName(kIOBlocksExtName));
+            if (iter != mExtensionBehavior.end())
+            {
+                iter->second = behaviorVal;
             }
         }
         return;

src/compiler/translator/ParseContext.cpp

@@ -240,6 +240,7 @@ TParseContext::TParseContext(TSymbolTable &symt,
       mGeometryShaderMaxVertices(-1),
       mMaxGeometryShaderInvocations(resources.MaxGeometryShaderInvocations),
       mMaxGeometryShaderMaxVertices(resources.MaxGeometryOutputVertices),
+      mGeometryInputArraySize(0),
       mFunctionBodyNewScope(false),
       mOutputType(outputType)
 {}
@@ -3071,6 +3072,7 @@ void TParseContext::setGeometryShaderInputArraySize(unsigned int inputArraySize,
               "array inputs.",
               "layout");
     }
+    mGeometryInputArraySize = inputArraySize;
 }
 
 bool TParseContext::parseGeometryShaderInputLayoutQualifier(const TTypeQualifier &typeQualifier)
@@ -3837,13 +3839,17 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
     TFieldList *fieldList,
     const ImmutableString &instanceName,
     const TSourceLoc &instanceLine,
-    TIntermTyped *arrayIndex,
-    const TSourceLoc &arrayIndexLine)
+    const TVector<unsigned int> *arraySizes,
+    const TSourceLoc &arraySizesLine)
 {
     checkIsNotReserved(nameLine, blockName);
 
     TTypeQualifier typeQualifier = typeQualifierBuilder.getVariableTypeQualifier(mDiagnostics);
 
+    const bool isUniformOrBuffer =
+        typeQualifier.qualifier == EvqUniform || typeQualifier.qualifier == EvqBuffer;
+    const bool isShaderIoBlock = IsShaderIoBlock(typeQualifier.qualifier);
+
     if (mShaderVersion < 310 && typeQualifier.qualifier != EvqUniform)
     {
         error(typeQualifier.line,
@@ -3853,7 +3859,7 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
     }
     else if (typeQualifier.qualifier != EvqUniform && typeQualifier.qualifier != EvqBuffer)
     {
-        if (IsShaderIoBlock(typeQualifier.qualifier))
+        if (isShaderIoBlock)
         {
             if (!isExtensionEnabled(TExtension::OES_shader_io_blocks) &&
                 !isExtensionEnabled(TExtension::EXT_shader_io_blocks))
@@ -3881,11 +3887,52 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
         checkMemoryQualifierIsNotSpecified(typeQualifier.memoryQualifier, typeQualifier.line);
     }
 
-    // add array index
-    unsigned int arraySize = 0;
-    if (arrayIndex != nullptr)
+    // Verify array sizes
+    if (arraySizes)
+    {
+        if (isUniformOrBuffer)
+        {
+            if (arraySizes->size() == 0)
+            {
+                error(arraySizesLine, "unsized arrays are not allowed with interface blocks", "");
+            }
+            if (arraySizes->size() > 1)
+            {
+                error(arraySizesLine, "array of arrays are not allowed with interface blocks", "");
+            }
+        }
+        else if (isShaderIoBlock)
+        {
+            size_t arrayDimensions = arraySizes->size();
+
+            // Geometry shader inputs have a level arrayness that must be ignored.
+            if (mShaderType == GL_GEOMETRY_SHADER_EXT && IsVaryingIn(typeQualifier.qualifier))
+            {
+                ASSERT(arrayDimensions > 0);
+                --arrayDimensions;
+
+                // Validate that the array size of input matches the geometry layout
+                // declaration, if not automatic (specified as []).
+                const unsigned int geometryDim = arraySizes->back();
+                if (geometryDim > 0 && geometryDim != mGeometryInputArraySize)
+                {
+                    error(arraySizesLine,
+                          "geometry shader input block array size inconsistent "
+                          "with primitive",
+                          "");
+                }
+            }
+
+            if (arrayDimensions > 1)
+            {
+                error(arraySizesLine, "array of arrays are not allowed with I/O blocks", "");
+            }
+        }
+    }
+    else if (isShaderIoBlock && mShaderType == GL_GEOMETRY_SHADER_EXT &&
+             IsVaryingIn(typeQualifier.qualifier))
     {
-        arraySize = checkIsValidArraySize(arrayIndexLine, arrayIndex);
+        error(arraySizesLine, "geometry shader input blocks must be an array", "");
     }
 
     checkIndexIsNotSpecified(typeQualifier.line, typeQualifier.layoutQualifier.index);
@@ -3896,6 +3943,8 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
     }
     else
     {
+        unsigned int arraySize =
+            arraySizes == nullptr || arraySizes->empty() ? 0 : (*arraySizes)[0];
         checkBlockBindingIsValid(typeQualifier.line, typeQualifier.qualifier,
                                  typeQualifier.layoutQualifier.binding, arraySize);
     }
@@ -4066,9 +4115,9 @@ TIntermDeclaration *TParseContext::addInterfaceBlock(
 
     TType *interfaceBlockType =
         new TType(interfaceBlock, typeQualifier.qualifier, blockLayoutQualifier);
-    if (arrayIndex != nullptr)
+    if (arraySizes)
     {
-        interfaceBlockType->makeArray(arraySize);
+        interfaceBlockType->makeArrays(*arraySizes);
     }
 
     // The instance variable gets created to refer to the interface block type from the AST
@@ -4240,8 +4289,12 @@ TIntermTyped *TParseContext::addIndexExpression(TIntermTyped *baseExpression,
                           "[");
                     break;
                 default:
-                    // We can reach here only in error cases.
-                    ASSERT(mDiagnostics->numErrors() > 0);
+                    // It's ok for shader I/O blocks to be dynamically indexed
+                    if (!IsShaderIoBlock(baseExpression->getQualifier()))
+                    {
+                        // We can reach here only in error cases.
+                        ASSERT(mDiagnostics->numErrors() > 0);
+                    }
                     break;
             }
         }

src/compiler/translator/ParseContext.h

@@ -358,8 +358,8 @@ class TParseContext : angle::NonCopyable
                                           TFieldList *fieldList,
                                           const ImmutableString &instanceName,
                                           const TSourceLoc &instanceLine,
-                                          TIntermTyped *arrayIndex,
-                                          const TSourceLoc &arrayIndexLine);
+                                          const TVector<unsigned int> *arraySizes,
+                                          const TSourceLoc &arraySizesLine);
 
     void parseLocalSize(const ImmutableString &qualifierType,
                         const TSourceLoc &qualifierTypeLine,
@@ -674,6 +674,7 @@ class TParseContext : angle::NonCopyable
     int mGeometryShaderMaxVertices;
     int mMaxGeometryShaderInvocations;
     int mMaxGeometryShaderMaxVertices;
+    unsigned int mGeometryInputArraySize;
 
     // Track when we add new scope for func body in ESSL 1.00 spec
     bool mFunctionBodyNewScope;

src/compiler/translator/glslang.y

@@ -618,9 +618,9 @@ declaration
         ES3_OR_NEWER(ImmutableString($2.string), @1, "interface blocks");
         $$ = context->addInterfaceBlock(*$1, @2, ImmutableString($2.string), $3, ImmutableString($5.string), @5, NULL, @$);
     }
-    | type_qualifier enter_struct struct_declaration_list RIGHT_BRACE IDENTIFIER LEFT_BRACKET constant_expression RIGHT_BRACKET SEMICOLON {
+    | type_qualifier enter_struct struct_declaration_list RIGHT_BRACE IDENTIFIER array_specifier SEMICOLON {
         ES3_OR_NEWER(ImmutableString($2.string), @1, "interface blocks");
-        $$ = context->addInterfaceBlock(*$1, @2, ImmutableString($2.string), $3, ImmutableString($5.string), @5, $7, @6);
+        $$ = context->addInterfaceBlock(*$1, @2, ImmutableString($2.string), $3, ImmutableString($5.string), @5, $6, @6);
     }
     | type_qualifier SEMICOLON {
         context->parseGlobalLayoutQualifier(*$1);

src/compiler/translator/util.cpp

@@ -551,6 +551,8 @@ bool IsVaryingOut(TQualifier qualifier)
         case EvqCentroidOut:
         case EvqVertexOut:
         case EvqGeometryOut:
+        case EvqTessControlOut:
+        case EvqTessEvaluationOut:
         case EvqSampleOut:
             return true;
 
@@ -572,6 +574,8 @@ bool IsVaryingIn(TQualifier qualifier)
         case EvqCentroidIn:
         case EvqFragmentIn:
         case EvqGeometryIn:
+        case EvqTessControlIn:
+        case EvqTessEvaluationIn:
         case EvqSampleIn:
             return true;
 

src/tests/deqp_support/deqp_gles31_test_expectations.txt

@@ -208,12 +208,6 @@
 // Shader I/O blocks:
 // Missing matching of block names with unnamed SSBOs with the same member variable
 3580 VULKAN : dEQP-GLES31.functional.shaders.linkage.es31.shader_storage_block.ambiguous_variable_name_3 = FAIL
-// Missing support for unsized geometry shader input I/O block arrays
-3580 VULKAN : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.rules.input_block = FAIL
-3580 VULKAN : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.types.float_array = FAIL
-// Incorrect error regarding interface block being in/out
-3580 VULKAN : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.rules.input_block_explicit_size = FAIL
-3580 VULKAN : dEQP-GLES31.functional.shaders.linkage.es31.geometry.varying.rules.output_block* = FAIL
 // Missing matching of struct name in member fields of matching nameless I/O blocks
 3580 VULKAN : dEQP-GLES31.functional.separate_shader.validation.es31.io_blocks.mismatch_different_member_struct_names = FAIL
 // Failed by glslang errors (matrix or packing qualifiers can only be used on a uniform or buffer)