Vulkan: Simplify RewriteCubeMapSamplersAs2DArray

src/compiler/translator/TranslatorVulkan.cpp

@@ -772,7 +772,8 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
     // - It dramatically simplifies future transformations w.r.t to samplers in structs, array of
     //   arrays of opaque types, atomic counters etc.
     // - Avoids the need for shader*ArrayDynamicIndexing Vulkan features.
-    if (!MonomorphizeUnsupportedFunctionsInVulkanGLSL(this, root, &getSymbolTable()))
+    if (!MonomorphizeUnsupportedFunctionsInVulkanGLSL(this, root, &getSymbolTable(),
+                                                      compileOptions))
     {
         return false;
     }

src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.cpp

@@ -239,9 +239,11 @@ class MonomorphizeTraverser final : public TIntermTraverser
   public:
     explicit MonomorphizeTraverser(TCompiler *compiler,
                                    TSymbolTable *symbolTable,
+                                   ShCompileOptions compileOptions,
                                    FunctionMap *functionMap)
         : TIntermTraverser(true, false, false, symbolTable),
           mCompiler(compiler),
+          mCompileOptions(compileOptions),
           mFunctionMap(functionMap)
     {}
 
@@ -314,17 +316,21 @@ class MonomorphizeTraverser final : public TIntermTraverser
             // - If the opaque uniform is an array of array of sampler or image, and it's partially
             //   subscripted (i.e. the function itself expects an array), or
             // - The opaque uniform is an atomic counter
+            // - The opaque uniform is a samplerCube and ES2's cube sampling emulation is requested.
             //
             const TType &type = uniform->getType();
             const bool isArrayOfArrayOfSamplerOrImage =
                 (type.isSampler() || type.isImage()) && type.isArrayOfArrays();
             const bool isParameterArrayOfOpaqueType = funcArgument->getType().isArray();
             const bool isAtomicCounter              = type.isAtomicCounter();
+            const bool isSamplerCubeEmulation =
+                type.isSamplerCube() &&
+                (mCompileOptions & SH_EMULATE_SEAMFUL_CUBE_MAP_SAMPLING) != 0;
 
             if (!(isStructContainingSamplers ||
                   (isSamplerInStruct && isParameterArrayOfOpaqueType) ||
                   (isArrayOfArrayOfSamplerOrImage && isParameterArrayOfOpaqueType) ||
-                  isAtomicCounter))
+                  isAtomicCounter || isSamplerCubeEmulation))
             {
                 continue;
             }
@@ -375,6 +381,7 @@ class MonomorphizeTraverser final : public TIntermTraverser
     }
 
     TCompiler *mCompiler;
+    ShCompileOptions mCompileOptions;
     bool mAnyMonomorphized = false;
 
     // Map of original to monomorphized functions.
@@ -491,7 +498,8 @@ void SortDeclarations(TIntermBlock *root)
 
 bool MonomorphizeUnsupportedFunctionsInVulkanGLSL(TCompiler *compiler,
                                                   TIntermBlock *root,
-                                                  TSymbolTable *symbolTable)
+                                                  TSymbolTable *symbolTable,
+                                                  ShCompileOptions compileOptions)
 {
     // First, sort out the declarations such that all non-function declarations are placed before
     // function definitions.  This way when the function is replaced with one that references said
@@ -503,7 +511,7 @@ bool MonomorphizeUnsupportedFunctionsInVulkanGLSL(TCompiler *compiler,
         FunctionMap functionMap;
         InitializeFunctionMap(root, &functionMap);
 
-        MonomorphizeTraverser monomorphizer(compiler, symbolTable, &functionMap);
+        MonomorphizeTraverser monomorphizer(compiler, symbolTable, compileOptions, &functionMap);
         root->traverse(&monomorphizer);
 
         if (!monomorphizer.getAnyMonomorphized())

src/compiler/translator/tree_ops/vulkan/MonomorphizeUnsupportedFunctionsInVulkanGLSL.h

@@ -11,6 +11,7 @@
 // - Partially subscripted array of array of samplers
 // - Partially subscripted array of array of images
 // - Atomic counters
+// - samplerCube variables when emulating ES2's cube map sampling
 //
 // This transformation basically duplicates such functions, removes the
 // sampler/image/atomic_counter parameters and uses the opaque uniforms used by the caller.
@@ -19,16 +20,17 @@
 #define COMPILER_TRANSLATOR_TREEOPS_VULKAN_MONOMORPHIZEUNSUPPORTEDFUNCTIONSINVULKANGLSL_H_
 
 #include "common/angleutils.h"
+#include "compiler/translator/Compiler.h"
 
 namespace sh
 {
-class TCompiler;
 class TIntermBlock;
 class TSymbolTable;
 
 ANGLE_NO_DISCARD bool MonomorphizeUnsupportedFunctionsInVulkanGLSL(TCompiler *compiler,
                                                                    TIntermBlock *root,
-                                                                   TSymbolTable *symbolTable);
+                                                                   TSymbolTable *symbolTable,
+                                                                   ShCompileOptions compileOptions);
 }  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_TREEOPS_VULKAN_MONOMORPHIZEUNSUPPORTEDFUNCTIONSINVULKANGLSL_H_

src/compiler/translator/tree_ops/vulkan/RewriteCubeMapSamplersAs2DArray.cpp

@@ -6,6 +6,9 @@
 // RewriteCubeMapSamplersAs2DArray: Change samplerCube samplers to sampler2DArray for seamful cube
 // map emulation.
 //
+// Relies on MonomorphizeUnsupportedFunctionsInVulkanGLSL to ensure samplerCube variables are not
+// passed to functions (for simplicity).
+//
 
 #include "compiler/translator/tree_ops/vulkan/RewriteCubeMapSamplersAs2DArray.h"
 
@@ -208,7 +211,7 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
 {
   public:
     RewriteCubeMapSamplersAs2DArrayTraverser(TSymbolTable *symbolTable, bool isFragmentShader)
-        : TIntermTraverser(true, true, true, symbolTable),
+        : TIntermTraverser(true, false, false, symbolTable),
           mCubeXYZToArrayUVL(nullptr),
           mCubeXYZToArrayUVLImplicit(nullptr),
           mIsFragmentShader(isFragmentShader),
@@ -218,11 +221,6 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
 
     bool visitDeclaration(Visit visit, TIntermDeclaration *node) override
     {
-        if (visit != PreVisit)
-        {
-            return true;
-        }
-
         const TIntermSequence &sequence = *(node->getSequence());
 
         TIntermTyped *variable = sequence.front()->getAsTyped();
@@ -242,90 +240,19 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
         return true;
     }
 
-    void visitFunctionPrototype(TIntermFunctionPrototype *node) override
-    {
-        const TFunction *function = node->getFunction();
-        // Go over the parameters and replace the samplerCube arguments with a sampler2DArray.
-        mRetyper.visitFunctionPrototype();
-        for (size_t paramIndex = 0; paramIndex < function->getParamCount(); ++paramIndex)
-        {
-            const TVariable *param = function->getParam(paramIndex);
-            TVariable *replacement = convertFunctionParameter(node, param);
-            if (replacement)
-            {
-                mRetyper.replaceFunctionParam(param, replacement);
-            }
-        }
-
-        TIntermFunctionPrototype *replacementPrototype =
-            mRetyper.convertFunctionPrototype(mSymbolTable, function);
-        if (replacementPrototype)
-        {
-            queueReplacement(replacementPrototype, OriginalNode::IS_DROPPED);
-        }
-    }
-
     bool visitAggregate(Visit visit, TIntermAggregate *node) override
     {
-        if (visit == PreVisit)
-        {
-            mRetyper.preVisitAggregate();
-        }
-
-        if (visit != PostVisit)
-        {
-            return true;
-        }
-
         if (node->getOp() == EOpCallBuiltInFunction)
         {
-            convertBuiltinFunction(node);
+            bool converted = convertBuiltinFunction(node);
+            return !converted;
         }
-        else if (node->getOp() == EOpCallFunctionInAST)
-        {
-            TIntermAggregate *substituteCall = mRetyper.convertASTFunction(node);
-            if (substituteCall)
-            {
-                queueReplacement(substituteCall, OriginalNode::IS_DROPPED);
-            }
-        }
-        mRetyper.postVisitAggregate();
 
+        // AST functions don't require modification as samplerCube function parameters are removed
+        // by MonomorphizeUnsupportedFunctionsInVulkanGLSL.
         return true;
     }
 
-    void visitSymbol(TIntermSymbol *symbol) override
-    {
-        if (!symbol->getType().isSamplerCube())
-        {
-            return;
-        }
-
-        const TVariable *samplerCubeVar = &symbol->variable();
-
-        TIntermTyped *sampler2DArrayVar =
-            new TIntermSymbol(mRetyper.getVariableReplacement(samplerCubeVar));
-        ASSERT(sampler2DArrayVar != nullptr);
-
-        TIntermNode *argument = symbol;
-
-        // We need to replace the whole function call argument with the symbol replaced.  The
-        // argument can either be the sampler (array) itself, or a subscript into a sampler array.
-        TIntermBinary *arrayExpression = getParentNode()->getAsBinaryNode();
-        if (arrayExpression)
-        {
-            ASSERT(arrayExpression->getOp() == EOpIndexDirect ||
-                   arrayExpression->getOp() == EOpIndexIndirect);
-
-            argument = arrayExpression;
-
-            sampler2DArrayVar = new TIntermBinary(arrayExpression->getOp(), sampler2DArrayVar,
-                                                  arrayExpression->getRight()->deepCopy());
-        }
-
-        mRetyper.replaceFunctionCallArg(argument, sampler2DArrayVar);
-    }
-
     TIntermFunctionDefinition *getCoordTranslationFunctionDecl()
     {
         return mCoordTranslationFunctionDecl;
@@ -356,18 +283,16 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
         TType *newType = new TType(samplerCubeVar->getType());
         newType->setBasicType(EbtSampler2DArray);
 
-        TVariable *sampler2DArrayVar =
-            new TVariable(mSymbolTable, samplerCubeVar->name(), newType, SymbolType::UserDefined);
+        TVariable *sampler2DArrayVar = new TVariable(mSymbolTable, samplerCubeVar->name(), newType,
+                                                     samplerCubeVar->symbolType());
 
         TIntermDeclaration *sampler2DArrayDecl = new TIntermDeclaration();
         sampler2DArrayDecl->appendDeclarator(new TIntermSymbol(sampler2DArrayVar));
 
-        TIntermSequence replacement;
-        replacement.push_back(sampler2DArrayDecl);
-        mMultiReplacements.emplace_back(getParentNode()->getAsBlock(), node, replacement);
+        queueReplacement(sampler2DArrayDecl, OriginalNode::IS_DROPPED);
 
         // Remember the sampler2DArray variable.
-        mRetyper.replaceGlobalVariable(samplerCubeVar, sampler2DArrayVar);
+        mSamplerMap[samplerCubeVar] = sampler2DArrayVar;
     }
 
     void declareCoordTranslationFunction(bool implicit,
@@ -774,28 +699,43 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
         return TIntermAggregate::CreateFunctionCall(*mCubeXYZToArrayUVLImplicit, args);
     }
 
-    TVariable *convertFunctionParameter(TIntermNode *parent, const TVariable *param)
+    TIntermTyped *getMappedSamplerExpression(TIntermNode *samplerCubeExpression)
     {
-        if (!param->getType().isSamplerCube())
+        // The argument passed to a function can either be the sampler, if not array, or a subscript
+        // into the sampler array.
+        TIntermSymbol *asSymbol = samplerCubeExpression->getAsSymbolNode();
+        TIntermBinary *asBinary = samplerCubeExpression->getAsBinaryNode();
+
+        if (asBinary)
         {
-            return nullptr;
+            // Only constant indexing is supported in ES2.0.
+            ASSERT(asBinary->getOp() == EOpIndexDirect);
+            asSymbol = asBinary->getLeft()->getAsSymbolNode();
         }
 
-        TType *newType = new TType(param->getType());
-        newType->setBasicType(EbtSampler2DArray);
+        // Arrays of arrays are not available in ES2.0.
+        ASSERT(asSymbol != nullptr);
+        const TVariable *samplerCubeVar = &asSymbol->variable();
 
-        TVariable *replacementVar =
-            new TVariable(mSymbolTable, param->name(), newType, SymbolType::UserDefined);
+        ASSERT(mSamplerMap.find(samplerCubeVar) != mSamplerMap.end());
+        const TVariable *mappedSamplerVar = mSamplerMap.at(samplerCubeVar);
 
-        return replacementVar;
+        TIntermTyped *mappedExpression = new TIntermSymbol(mappedSamplerVar);
+        if (asBinary)
+        {
+            mappedExpression =
+                new TIntermBinary(asBinary->getOp(), mappedExpression, asBinary->getRight());
+        }
+
+        return mappedExpression;
     }
 
-    void convertBuiltinFunction(TIntermAggregate *node)
+    bool convertBuiltinFunction(TIntermAggregate *node)
     {
         const TFunction *function = node->getFunction();
         if (!function->name().beginsWith("textureCube"))
         {
-            return;
+            return false;
         }
 
         // All textureCube* functions are in the form:
@@ -950,7 +890,7 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
 
         TIntermSequence *substituteArguments = new TIntermSequence;
         // Replace the first argument (samplerCube) with the sampler2DArray.
-        substituteArguments->push_back(mRetyper.getFunctionCallArgReplacement((*arguments)[0]));
+        substituteArguments->push_back(getMappedSamplerExpression((*arguments)[0]));
         // Replace the second argument with the coordination transformation.
         substituteArguments->push_back(uvl->deepCopy());
         if (isTranslatedGrad)
@@ -971,9 +911,12 @@ class RewriteCubeMapSamplersAs2DArrayTraverser : public TIntermTraverser
             substituteFunctionName, substituteArguments, *mSymbolTable, 300);
 
         queueReplacement(substituteCall, OriginalNode::IS_DROPPED);
+
+        return true;
     }
 
-    RetypeOpaqueVariablesHelper mRetyper;
+    // A map from the samplerCube variable to the sampler2DArray one.
+    angle::HashMap<const TVariable *, const TVariable *> mSamplerMap;
 
     // A helper function to convert xyz coordinates passed to a cube map sampling function into the
     // array layer (cube map face) and uv coordinates.

src/compiler/translator/tree_util/ReplaceVariable.cpp

@@ -142,75 +142,4 @@ ANGLE_NO_DISCARD bool ReplaceVariableWithTyped(TCompiler *compiler,
     return traverser.updateTree(compiler, root);
 }
 
-TIntermFunctionPrototype *RetypeOpaqueVariablesHelper::convertFunctionPrototype(
-    TSymbolTable *symbolTable,
-    const TFunction *oldFunction)
-{
-    if (mReplacedFunctionParams.empty())
-    {
-        return nullptr;
-    }
-
-    // Create a new function prototype for replacement.
-    TFunction *replacementFunction = new TFunction(
-        symbolTable, oldFunction->name(), SymbolType::UserDefined,
-        new TType(oldFunction->getReturnType()), oldFunction->isKnownToNotHaveSideEffects());
-    for (size_t paramIndex = 0; paramIndex < oldFunction->getParamCount(); ++paramIndex)
-    {
-        const TVariable *param = oldFunction->getParam(paramIndex);
-        TVariable *replacement = nullptr;
-        auto replaced          = mReplacedFunctionParams.find(param);
-        if (replaced != mReplacedFunctionParams.end())
-        {
-            replacement = replaced->second;
-        }
-        else
-        {
-            replacement = new TVariable(symbolTable, param->name(), new TType(param->getType()),
-                                        SymbolType::UserDefined);
-        }
-        replacementFunction->addParameter(replacement);
-    }
-    mReplacedFunctions[oldFunction] = replacementFunction;
-
-    TIntermFunctionPrototype *replacementPrototype =
-        new TIntermFunctionPrototype(replacementFunction);
-
-    return replacementPrototype;
-}
-
-TIntermAggregate *RetypeOpaqueVariablesHelper::convertASTFunction(TIntermAggregate *node)
-{
-    // See if the function needs replacement at all.
-    const TFunction *function = node->getFunction();
-    auto replacedFunction     = mReplacedFunctions.find(function);
-    if (replacedFunction == mReplacedFunctions.end())
-    {
-        return nullptr;
-    }
-
-    // Arguments to this call are staged to be replaced at the same time.
-    TFunction *substituteFunction        = replacedFunction->second;
-    TIntermSequence *substituteArguments = new TIntermSequence;
-
-    for (size_t paramIndex = 0; paramIndex < function->getParamCount(); ++paramIndex)
-    {
-        TIntermNode *param = node->getChildNode(paramIndex);
-
-        TIntermNode *replacement = nullptr;
-        auto replacedArg         = mReplacedFunctionCallArgs.top().find(param);
-        if (replacedArg != mReplacedFunctionCallArgs.top().end())
-        {
-            replacement = replacedArg->second;
-        }
-        else
-        {
-            replacement = param->getAsTyped()->deepCopy();
-        }
-        substituteArguments->push_back(replacement);
-    }
-
-    return TIntermAggregate::CreateFunctionCall(*substituteFunction, substituteArguments);
-}
-
 }  // namespace sh

src/compiler/translator/tree_util/ReplaceVariable.h

@@ -50,108 +50,6 @@ void GetDeclaratorReplacements(TSymbolTable *symbolTable,
                                TIntermBlock *root,
                                VariableReplacementMap *variableMap);
 
-// A helper class to keep track of opaque variable re-typing during a pass.  Unlike the above
-// functions, this can be used to replace all opaque variables of a certain type with another in a
-// pass that possibly does other related transformations.  Only opaque variables are supported as
-// replacing local variables is not supported.
-//
-// The class uses "old" to refer to the original type of the variable and "new" to refer to the type
-// that will replace it.
-//
-// - replaceGlobalVariable(): Call to track a global variable that is replaced.
-// - in TIntermTraverser::visitFunctionPrototype():
-//   * Call visitFunctionPrototype().
-//   * For every replaced parameter, call replaceFunctionParam().
-//   * call convertFunctionPrototype() to convert the prototype based on the above replacements
-//     and track the function with its replacement.
-//   * Call replaceFunction() to track the function that is replaced.
-// - In PreVisit of TIntermTraverser::visitAggregate():
-//   * call preVisitAggregate()
-// - In TIntermTraverser::visitSymbol():
-//   * Replace non-function-call-argument symbols that refer to a global or function param with the
-//     replacement (getVariableReplacement()).
-//   * For function call arguments, call replaceFunctionCallArg() to track the replacement.
-// - In PostVisit of TIntermTraverser::visitAggregate():
-//   * Convert built-in functions per case.  Call convertASTFunction() for non built-in functions
-//     for the replacement to be created.
-//   * Call postVisitAggregate() when done.
-//
-class RetypeOpaqueVariablesHelper
-{
-  public:
-    RetypeOpaqueVariablesHelper() {}
-    ~RetypeOpaqueVariablesHelper() {}
-
-    // Global variable handling:
-    void replaceGlobalVariable(const TVariable *oldVar, TVariable *newVar)
-    {
-        ASSERT(mReplacedGlobalVariables.count(oldVar) == 0);
-        mReplacedGlobalVariables[oldVar] = newVar;
-    }
-    TVariable *getVariableReplacement(const TVariable *oldVar) const
-    {
-        if (mReplacedGlobalVariables.count(oldVar) != 0)
-        {
-            return mReplacedGlobalVariables.at(oldVar);
-        }
-        else
-        {
-            // This function should only be called if the variable is expected to have been
-            // replaced either way (as a global variable or a function parameter).
-            ASSERT(mReplacedFunctionParams.count(oldVar) != 0);
-            return mReplacedFunctionParams.at(oldVar);
-        }
-    }
-
-    // Function parameters handling:
-    void visitFunctionPrototype() { mReplacedFunctionParams.clear(); }
-    void replaceFunctionParam(const TVariable *oldParam, TVariable *newParam)
-    {
-        ASSERT(mReplacedFunctionParams.count(oldParam) == 0);
-        mReplacedFunctionParams[oldParam] = newParam;
-    }
-    TVariable *getFunctionParamReplacement(const TVariable *oldParam) const
-    {
-        ASSERT(mReplacedFunctionParams.count(oldParam) != 0);
-        return mReplacedFunctionParams.at(oldParam);
-    }
-
-    // Function call arguments handling:
-    void preVisitAggregate() { mReplacedFunctionCallArgs.emplace(); }
-    bool isInAggregate() const { return !mReplacedFunctionCallArgs.empty(); }
-    void postVisitAggregate() { mReplacedFunctionCallArgs.pop(); }
-    void replaceFunctionCallArg(const TIntermNode *oldArg, TIntermTyped *newArg)
-    {
-        ASSERT(mReplacedFunctionCallArgs.top().count(oldArg) == 0);
-        mReplacedFunctionCallArgs.top()[oldArg] = newArg;
-    }
-    TIntermTyped *getFunctionCallArgReplacement(const TIntermNode *oldArg) const
-    {
-        ASSERT(mReplacedFunctionCallArgs.top().count(oldArg) != 0);
-        return mReplacedFunctionCallArgs.top().at(oldArg);
-    }
-
-    // Helper code conversion methods.
-    TIntermFunctionPrototype *convertFunctionPrototype(TSymbolTable *symbolTable,
-                                                       const TFunction *oldFunction);
-    TIntermAggregate *convertASTFunction(TIntermAggregate *node);
-
-  private:
-    // A map from the old global variable to the new one.
-    angle::HashMap<const TVariable *, TVariable *> mReplacedGlobalVariables;
-
-    // A map from functions with old type parameters to one where that's replaced with the new type.
-    angle::HashMap<const TFunction *, TFunction *> mReplacedFunctions;
-
-    // A map from function old type parameters to their replacement new type parameter for the
-    // current function definition.
-    angle::HashMap<const TVariable *, TVariable *> mReplacedFunctionParams;
-
-    // A map from function call old type arguments to their replacement for the current function
-    // call.
-    std::stack<angle::HashMap<const TIntermNode *, TIntermTyped *>> mReplacedFunctionCallArgs;
-};
-
 }  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_TREEUTIL_REPLACEVARIABLE_H_