Reland "Make SH_CLAMP_INDIRECT_ARRAY_BOUNDS do proper AST transformation"

include/GLSLANG/ShaderLang.h

@@ -26,7 +26,7 @@
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 259
+#define ANGLE_SH_VERSION 260
 
 enum ShShaderSpec
 {
@@ -123,10 +123,7 @@ const ShCompileOptions SH_ENFORCE_PACKING_RESTRICTIONS = UINT64_C(1) << 9;
 // This flag ensures all indirect (expression-based) array indexing
 // is clamped to the bounds of the array. This ensures, for example,
 // that you cannot read off the end of a uniform, whether an array
-// vec234, or mat234 type. The ShArrayIndexClampingStrategy enum,
-// specified in the ShBuiltInResources when constructing the
-// compiler, selects the strategy for the clamping implementation.
-// TODO(http://anglebug.com/4361): fix for compute shaders.
+// vec234, or mat234 type.
 const ShCompileOptions SH_CLAMP_INDIRECT_ARRAY_BOUNDS = UINT64_C(1) << 10;
 
 // This flag limits the complexity of an expression.
@@ -344,16 +341,6 @@ const ShCompileOptions SH_INIT_FRAGMENT_OUTPUT_VARIABLES = UINT64_C(1) << 57;
 // non-dcheck-enabled builds to avoid increasing ANGLE's binary size while both generators coexist.
 const ShCompileOptions SH_GENERATE_SPIRV_DIRECTLY = UINT64_C(1) << 58;
 
-// Defines alternate strategies for implementing array index clamping.
-enum ShArrayIndexClampingStrategy
-{
-    // Use the clamp intrinsic for array index clamping.
-    SH_CLAMP_WITH_CLAMP_INTRINSIC = 1,
-
-    // Use a user-defined function for array index clamping.
-    SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION
-};
-
 // The 64 bits hash function. The first parameter is the input string; the
 // second parameter is the string length.
 using ShHashFunction64 = khronos_uint64_t (*)(const char *, size_t);
@@ -452,10 +439,6 @@ struct ShBuiltInResources
     // Default is NULL.
     ShHashFunction64 HashFunction;
 
-    // Selects a strategy to use when implementing array index clamping.
-    // Default is SH_CLAMP_WITH_CLAMP_INTRINSIC.
-    ShArrayIndexClampingStrategy ArrayIndexClampingStrategy;
-
     // The maximum complexity an expression can be when SH_LIMIT_EXPRESSION_COMPLEXITY is turned on.
     int MaxExpressionComplexity;
 

src/compiler.gni

@@ -123,6 +123,8 @@ angle_translator_sources = [
   "src/compiler/translator/glslang_tab_autogen.h",
   "src/compiler/translator/glslang_wrapper.h",
   "src/compiler/translator/length_limits.h",
+  "src/compiler/translator/tree_ops/ClampIndirectIndices.cpp",
+  "src/compiler/translator/tree_ops/ClampIndirectIndices.h",
   "src/compiler/translator/tree_ops/ClampPointSize.cpp",
   "src/compiler/translator/tree_ops/ClampPointSize.h",
   "src/compiler/translator/tree_ops/DeclareAndInitBuiltinsForInstancedMultiview.cpp",
@@ -227,8 +229,6 @@ angle_translator_sources = [
   "src/compiler/translator/tree_util/Visit.h",
   "src/compiler/translator/util.cpp",
   "src/compiler/translator/util.h",
-  "src/third_party/compiler/ArrayBoundsClamper.cpp",
-  "src/third_party/compiler/ArrayBoundsClamper.h",
 ]
 angle_translator_glsl_base_sources = [
   "src/compiler/translator/OutputGLSLBase.cpp",

src/compiler/translator/Compiler.cpp

@@ -23,6 +23,7 @@
 #include "compiler/translator/ValidateOutputs.h"
 #include "compiler/translator/ValidateVaryingLocations.h"
 #include "compiler/translator/VariablePacker.h"
+#include "compiler/translator/tree_ops/ClampIndirectIndices.h"
 #include "compiler/translator/tree_ops/ClampPointSize.h"
 #include "compiler/translator/tree_ops/DeclareAndInitBuiltinsForInstancedMultiview.h"
 #include "compiler/translator/tree_ops/DeferGlobalInitializers.h"
@@ -55,7 +56,6 @@
 #include "compiler/translator/tree_util/IntermNodePatternMatcher.h"
 #include "compiler/translator/tree_util/ReplaceShadowingVariables.h"
 #include "compiler/translator/util.h"
-#include "third_party/compiler/ArrayBoundsClamper.h"
 
 namespace sh
 {
@@ -336,7 +336,6 @@ bool TCompiler::Init(const ShBuiltInResources &resources)
     setResourceString();
 
     InitExtensionBehavior(resources, mExtensionBehavior);
-    mArrayBoundsClamper.SetClampingStrategy(resources.ArrayIndexClampingStrategy);
     return true;
 }
 
@@ -690,7 +689,10 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
     // Clamping uniform array bounds needs to happen after validateLimitations pass.
     if ((compileOptions & SH_CLAMP_INDIRECT_ARRAY_BOUNDS) != 0)
     {
-        mArrayBoundsClamper.MarkIndirectArrayBoundsForClamping(root);
+        if (!ClampIndirectIndices(this, root, &mSymbolTable))
+        {
+            return false;
+        }
     }
 
     if ((compileOptions & SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW) != 0 &&
@@ -1264,7 +1266,6 @@ bool TCompiler::emulatePrecisionIfNeeded(TIntermBlock *root,
 
 void TCompiler::clearResults()
 {
-    mArrayBoundsClamper.Cleanup();
     mInfoSink.info.erase();
     mInfoSink.obj.erase();
     mInfoSink.debug.erase();
@@ -1561,16 +1562,6 @@ const ShBuiltInResources &TCompiler::getResources() const
     return mResources;
 }
 
-const ArrayBoundsClamper &TCompiler::getArrayBoundsClamper() const
-{
-    return mArrayBoundsClamper;
-}
-
-ShArrayIndexClampingStrategy TCompiler::getArrayIndexClampingStrategy() const
-{
-    return mResources.ArrayIndexClampingStrategy;
-}
-
 const BuiltInFunctionEmulator &TCompiler::getBuiltInFunctionEmulator() const
 {
     return mBuiltInFunctionEmulator;

src/compiler/translator/Compiler.h

@@ -26,7 +26,6 @@
 #include "compiler/translator/Pragma.h"
 #include "compiler/translator/SymbolTable.h"
 #include "compiler/translator/ValidateAST.h"
-#include "third_party/compiler/ArrayBoundsClamper.h"
 
 namespace sh
 {
@@ -196,8 +195,6 @@ class TCompiler : public TShHandleBase
     // Relies on collectVariables having been called.
     bool isVaryingDefined(const char *varyingName);
 
-    const ArrayBoundsClamper &getArrayBoundsClamper() const;
-    ShArrayIndexClampingStrategy getArrayIndexClampingStrategy() const;
     const BuiltInFunctionEmulator &getBuiltInFunctionEmulator() const;
 
     virtual bool shouldFlattenPragmaStdglInvariantAll() = 0;
@@ -301,7 +298,6 @@ class TCompiler : public TShHandleBase
     // Built-in extensions with default behavior.
     TExtensionBehavior mExtensionBehavior;
 
-    ArrayBoundsClamper mArrayBoundsClamper;
     BuiltInFunctionEmulator mBuiltInFunctionEmulator;
 
     // Results of compilation.

src/compiler/translator/IntermNode.cpp

@@ -1112,8 +1112,7 @@ TIntermSwizzle::TIntermSwizzle(const TIntermSwizzle &node) : TIntermExpression(n
     mHasFoldedDuplicateOffsets = node.mHasFoldedDuplicateOffsets;
 }
 
-TIntermBinary::TIntermBinary(const TIntermBinary &node)
-    : TIntermOperator(node), mAddIndexClamp(node.mAddIndexClamp)
+TIntermBinary::TIntermBinary(const TIntermBinary &node) : TIntermOperator(node)
 {
     TIntermTyped *leftCopy  = node.mLeft->deepCopy();
     TIntermTyped *rightCopy = node.mRight->deepCopy();
@@ -1374,7 +1373,7 @@ TIntermUnary::TIntermUnary(TOperator op, TIntermTyped *operand, const TFunction
 }
 
 TIntermBinary::TIntermBinary(TOperator op, TIntermTyped *left, TIntermTyped *right)
-    : TIntermOperator(op), mLeft(left), mRight(right), mAddIndexClamp(false)
+    : TIntermOperator(op), mLeft(left), mRight(right)
 {
     ASSERT(mLeft);
     ASSERT(mRight);

src/compiler/translator/IntermNode.h

@@ -495,9 +495,6 @@ class TIntermBinary : public TIntermOperator
     TIntermTyped *getRight() const { return mRight; }
     TIntermTyped *fold(TDiagnostics *diagnostics) override;
 
-    void setAddIndexClamp() { mAddIndexClamp = true; }
-    bool getAddIndexClamp() const { return mAddIndexClamp; }
-
     // This method is only valid for EOpIndexDirectStruct. It returns the name of the field.
     const ImmutableString &getIndexStructFieldName() const;
 
@@ -505,9 +502,6 @@ class TIntermBinary : public TIntermOperator
     TIntermTyped *mLeft;
     TIntermTyped *mRight;
 
-    // If set to true, wrap any EOpIndexIndirect with a clamp to bounds.
-    bool mAddIndexClamp;
-
   private:
     void promote();
 

src/compiler/translator/OutputESSL.cpp

@@ -10,7 +10,6 @@ namespace sh
 {
 
 TOutputESSL::TOutputESSL(TInfoSinkBase &objSink,
-                         ShArrayIndexClampingStrategy clampingStrategy,
                          ShHashFunction64 hashFunction,
                          NameMap &nameMap,
                          TSymbolTable *symbolTable,
@@ -19,7 +18,6 @@ TOutputESSL::TOutputESSL(TInfoSinkBase &objSink,
                          bool forceHighp,
                          ShCompileOptions compileOptions)
     : TOutputGLSLBase(objSink,
-                      clampingStrategy,
                       hashFunction,
                       nameMap,
                       symbolTable,

src/compiler/translator/OutputESSL.h

@@ -16,7 +16,6 @@ class TOutputESSL : public TOutputGLSLBase
 {
   public:
     TOutputESSL(TInfoSinkBase &objSink,
-                ShArrayIndexClampingStrategy clampingStrategy,
                 ShHashFunction64 hashFunction,
                 NameMap &nameMap,
                 TSymbolTable *symbolTable,

src/compiler/translator/OutputGLSL.cpp

@@ -12,7 +12,6 @@ namespace sh
 {
 
 TOutputGLSL::TOutputGLSL(TInfoSinkBase &objSink,
-                         ShArrayIndexClampingStrategy clampingStrategy,
                          ShHashFunction64 hashFunction,
                          NameMap &nameMap,
                          TSymbolTable *symbolTable,
@@ -21,7 +20,6 @@ TOutputGLSL::TOutputGLSL(TInfoSinkBase &objSink,
                          ShShaderOutput output,
                          ShCompileOptions compileOptions)
     : TOutputGLSLBase(objSink,
-                      clampingStrategy,
                       hashFunction,
                       nameMap,
                       symbolTable,

src/compiler/translator/OutputGLSL.h

@@ -16,7 +16,6 @@ class TOutputGLSL : public TOutputGLSLBase
 {
   public:
     TOutputGLSL(TInfoSinkBase &objSink,
-                ShArrayIndexClampingStrategy clampingStrategy,
                 ShHashFunction64 hashFunction,
                 NameMap &nameMap,
                 TSymbolTable *symbolTable,

src/compiler/translator/OutputGLSLBase.cpp

@@ -82,7 +82,6 @@ Stream &operator<<(Stream &out, CommaSeparatedListItemPrefixGenerator &gen)
 }  // namespace
 
 TOutputGLSLBase::TOutputGLSLBase(TInfoSinkBase &objSink,
-                                 ShArrayIndexClampingStrategy clampingStrategy,
                                  ShHashFunction64 hashFunction,
                                  NameMap &nameMap,
                                  TSymbolTable *symbolTable,
@@ -93,7 +92,6 @@ TOutputGLSLBase::TOutputGLSLBase(TInfoSinkBase &objSink,
     : TIntermTraverser(true, true, true, symbolTable),
       mObjSink(objSink),
       mDeclaringVariable(false),
-      mClampingStrategy(clampingStrategy),
       mHashFunction(hashFunction),
       mNameMap(nameMap),
       mShaderType(shaderType),
@@ -597,62 +595,8 @@ bool TOutputGLSLBase::visitBinary(Visit visit, TIntermBinary *node)
             break;
 
         case EOpIndexDirect:
-            writeTriplet(visit, nullptr, "[", "]");
-            break;
         case EOpIndexIndirect:
-            if (node->getAddIndexClamp())
-            {
-                if (visit == InVisit)
-                {
-                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
-                        out << "[int(clamp(float(";
-                    else
-                        out << "[webgl_int_clamp(";
-                }
-                else if (visit == PostVisit)
-                {
-                    TIntermTyped *left = node->getLeft();
-                    TType leftType     = left->getType();
-
-                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
-                        out << "), 0.0, float(";
-                    else
-                        out << ", 0, ";
-
-                    if (leftType.isUnsizedArray())
-                    {
-                        // For runtime-sized arrays in ESSL 3.10 we need to call the length method
-                        // to get the length to clamp against. See ESSL 3.10 section 4.1.9. Note
-                        // that a runtime-sized array expression is guaranteed not to have side
-                        // effects, so it's fine to add the expression to the output twice.
-                        ASSERT(mShaderVersion >= 310);
-                        ASSERT(!left->hasSideEffects());
-                        left->traverse(this);
-                        out << ".length() - 1";
-                    }
-                    else
-                    {
-                        int maxSize;
-                        if (leftType.isArray())
-                        {
-                            maxSize = static_cast<int>(leftType.getOutermostArraySize()) - 1;
-                        }
-                        else
-                        {
-                            maxSize = leftType.getNominalSize() - 1;
-                        }
-                        out << maxSize;
-                    }
-                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC)
-                        out << ")))]";
-                    else
-                        out << ")]";
-                }
-            }
-            else
-            {
-                writeTriplet(visit, nullptr, "[", "]");
-            }
+            writeTriplet(visit, nullptr, "[", "]");
             break;
         case EOpIndexDirectStruct:
             if (visit == InVisit)

src/compiler/translator/OutputGLSLBase.h

@@ -20,7 +20,6 @@ class TOutputGLSLBase : public TIntermTraverser
 {
   public:
     TOutputGLSLBase(TInfoSinkBase &objSink,
-                    ShArrayIndexClampingStrategy clampingStrategy,
                     ShHashFunction64 hashFunction,
                     NameMap &nameMap,
                     TSymbolTable *symbolTable,
@@ -103,8 +102,6 @@ class TOutputGLSLBase : public TIntermTraverser
     TInfoSinkBase &mObjSink;
     bool mDeclaringVariable;
 
-    ShArrayIndexClampingStrategy mClampingStrategy;
-
     // name hashing.
     ShHashFunction64 mHashFunction;
 

src/compiler/translator/OutputVulkanGLSL.cpp

@@ -20,7 +20,6 @@ namespace sh
 {
 
 TOutputVulkanGLSL::TOutputVulkanGLSL(TInfoSinkBase &objSink,
-                                     ShArrayIndexClampingStrategy clampingStrategy,
                                      ShHashFunction64 hashFunction,
                                      NameMap &nameMap,
                                      TSymbolTable *symbolTable,
@@ -31,7 +30,6 @@ TOutputVulkanGLSL::TOutputVulkanGLSL(TInfoSinkBase &objSink,
                                      bool enablePrecision,
                                      ShCompileOptions compileOptions)
     : TOutputGLSL(objSink,
-                  clampingStrategy,
                   hashFunction,
                   nameMap,
                   symbolTable,

src/compiler/translator/OutputVulkanGLSL.h

@@ -21,7 +21,6 @@ class TOutputVulkanGLSL : public TOutputGLSL
 {
   public:
     TOutputVulkanGLSL(TInfoSinkBase &objSink,
-                      ShArrayIndexClampingStrategy clampingStrategy,
                       ShHashFunction64 hashFunction,
                       NameMap &nameMap,
                       TSymbolTable *symbolTable,

src/compiler/translator/OutputVulkanGLSLForMetal.h

@@ -20,7 +20,6 @@ class TOutputVulkanGLSLForMetal : public TOutputVulkanGLSL
 {
   public:
     TOutputVulkanGLSLForMetal(TInfoSinkBase &objSink,
-                              ShArrayIndexClampingStrategy clampingStrategy,
                               ShHashFunction64 hashFunction,
                               NameMap &nameMap,
                               TSymbolTable *symbolTable,

src/compiler/translator/OutputVulkanGLSLForMetal.mm

@@ -55,7 +55,6 @@ void TOutputVulkanGLSLForMetal::RemoveInvariantForTest(bool remove)
 }
 
 TOutputVulkanGLSLForMetal::TOutputVulkanGLSLForMetal(TInfoSinkBase &objSink,
-                                                     ShArrayIndexClampingStrategy clampingStrategy,
                                                      ShHashFunction64 hashFunction,
                                                      NameMap &nameMap,
                                                      TSymbolTable *symbolTable,
@@ -64,7 +63,6 @@ TOutputVulkanGLSLForMetal::TOutputVulkanGLSLForMetal(TInfoSinkBase &objSink,
                                                      ShShaderOutput output,
                                                      ShCompileOptions compileOptions)
     : TOutputVulkanGLSL(objSink,
-                        clampingStrategy,
                         hashFunction,
                         nameMap,
                         symbolTable,

src/compiler/translator/ShaderLang.cpp

@@ -244,8 +244,6 @@ void InitBuiltInResources(ShBuiltInResources *resources)
     // Disable name hashing by default.
     resources->HashFunction = nullptr;
 
-    resources->ArrayIndexClampingStrategy = SH_CLAMP_WITH_CLAMP_INTRINSIC;
-
     resources->MaxExpressionComplexity = 256;
     resources->MaxCallStackDepth       = 256;
     resources->MaxFunctionParameters   = 1024;

src/compiler/translator/TranslatorESSL.cpp

@@ -76,9 +76,6 @@ bool TranslatorESSL::translate(TIntermBlock *root,
         sink << "// END: Generated code for built-in function emulation\n\n";
     }
 
-    // Write array bounds clamping emulation if needed.
-    getArrayBoundsClamper().OutputClampingFunctionDefinition(sink);
-
     if (getShaderType() == GL_FRAGMENT_SHADER)
     {
         EmitEarlyFragmentTestsGLSL(*this, sink);
@@ -97,9 +94,8 @@ bool TranslatorESSL::translate(TIntermBlock *root,
     }
 
     // Write translated shader.
-    TOutputESSL outputESSL(sink, getArrayIndexClampingStrategy(), getHashFunction(), getNameMap(),
-                           &getSymbolTable(), getShaderType(), shaderVer, precisionEmulation,
-                           compileOptions);
+    TOutputESSL outputESSL(sink, getHashFunction(), getNameMap(), &getSymbolTable(),
+                           getShaderType(), shaderVer, precisionEmulation, compileOptions);
 
     root->traverse(&outputESSL);
 

src/compiler/translator/TranslatorGLSL.cpp

@@ -130,9 +130,6 @@ bool TranslatorGLSL::translate(TIntermBlock *root,
         sink << "// END: Generated code for built-in function emulation\n\n";
     }
 
-    // Write array bounds clamping emulation if needed.
-    getArrayBoundsClamper().OutputClampingFunctionDefinition(sink);
-
     // Declare gl_FragColor and glFragData as webgl_FragColor and webgl_FragData
     // if it's core profile shaders and they are used.
     if (getShaderType() == GL_FRAGMENT_SHADER)
@@ -216,9 +213,8 @@ bool TranslatorGLSL::translate(TIntermBlock *root,
     }
 
     // Write translated shader.
-    TOutputGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(), getNameMap(),
-                           &getSymbolTable(), getShaderType(), getShaderVersion(), getOutputType(),
-                           compileOptions);
+    TOutputGLSL outputGLSL(sink, getHashFunction(), getNameMap(), &getSymbolTable(),
+                           getShaderType(), getShaderVersion(), getOutputType(), compileOptions);
 
     root->traverse(&outputGLSL);
 

src/compiler/translator/TranslatorMetal.cpp

@@ -240,9 +240,9 @@ bool TranslatorMetal::translate(TIntermBlock *root,
     }
 
     // Write translated shader.
-    TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
-                                 getNameMap(), &getSymbolTable(), getShaderType(),
-                                 getShaderVersion(), getOutputType(), false, true, compileOptions);
+    TOutputVulkanGLSL outputGLSL(sink, getHashFunction(), getNameMap(), &getSymbolTable(),
+                                 getShaderType(), getShaderVersion(), getOutputType(), false, true,
+                                 compileOptions);
     root->traverse(&outputGLSL);
 
     return compileToSpirv(sink);

src/compiler/translator/TranslatorVulkan.cpp

@@ -1357,10 +1357,9 @@ bool TranslatorVulkan::translate(TIntermBlock *root,
 #endif
 
     // Write translated shader.
-    TOutputVulkanGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(),
-                                 getNameMap(), &getSymbolTable(), getShaderType(),
-                                 getShaderVersion(), getOutputType(), precisionEmulation,
-                                 enablePrecision, compileOptions);
+    TOutputVulkanGLSL outputGLSL(sink, getHashFunction(), getNameMap(), &getSymbolTable(),
+                                 getShaderType(), getShaderVersion(), getOutputType(),
+                                 precisionEmulation, enablePrecision, compileOptions);
     root->traverse(&outputGLSL);
 
     return compileToSpirv(sink);

src/compiler/translator/tree_ops/ClampIndirectIndices.cpp

+//
+// Copyright 2021 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// ClampIndirectIndices.h: Add clamp to the indirect indices used on arrays.
+//
+
+#include "compiler/translator/tree_ops/ClampIndirectIndices.h"
+
+#include "compiler/translator/Compiler.h"
+#include "compiler/translator/StaticType.h"
+#include "compiler/translator/SymbolTable.h"
+#include "compiler/translator/tree_util/IntermNode_util.h"
+#include "compiler/translator/tree_util/IntermTraverse.h"
+
+namespace sh
+{
+namespace
+{
+// Traverser that finds EOpIndexIndirect nodes and applies a clamp to their right-hand side
+// expression.
+class ClampIndirectIndicesTraverser : public TIntermTraverser
+{
+  public:
+    ClampIndirectIndicesTraverser(TCompiler *compiler, TSymbolTable *symbolTable)
+        : TIntermTraverser(true, false, false, symbolTable), mCompiler(compiler)
+    {}
+
+    bool visitBinary(Visit visit, TIntermBinary *node) override
+    {
+        ASSERT(visit == PreVisit);
+
+        // Only interested in EOpIndexIndirect nodes.
+        if (node->getOp() != EOpIndexIndirect)
+        {
+            return true;
+        }
+
+        // Apply the transformation to the left and right nodes
+        bool valid = ClampIndirectIndices(mCompiler, node->getLeft(), mSymbolTable);
+        ASSERT(valid);
+        valid = ClampIndirectIndices(mCompiler, node->getRight(), mSymbolTable);
+        ASSERT(valid);
+
+        // Generate clamp(right, 0, N), where N is the size of the array being indexed minus 1.  If
+        // the array is runtime-sized, the length() method is called on it.
+        //
+        // Note that on GLSL es 100, clamp is only defined for float, so float arguments are used.
+        const TType &leftType    = node->getLeft()->getType();
+        const TType &rightType   = node->getRight()->getType();
+        const bool useFloatClamp = mCompiler->getShaderVersion() == 100;
+
+        TIntermConstantUnion *zero = createClampValue(0, useFloatClamp);
+        TIntermTyped *max;
+
+        if (leftType.isUnsizedArray())
+        {
+            // Unsized arrays are an ES3.1 feature, so integer clamp should be available already.
+            max = new TIntermUnary(EOpArrayLength, node->getLeft(), nullptr);
+            max = new TIntermBinary(EOpSub, max, CreateIndexNode(1));
+        }
+        else if (leftType.isArray())
+        {
+            max = createClampValue(static_cast<int>(leftType.getOutermostArraySize()) - 1,
+                                   useFloatClamp);
+        }
+        else
+        {
+            ASSERT(leftType.isVector() || leftType.isMatrix());
+            max = createClampValue(leftType.getNominalSize() - 1, useFloatClamp);
+        }
+
+        TIntermTyped *index = node->getRight();
+        // If the index node is not an int (i.e. it's a uint), or a float (if using float clamp),
+        // cast it.
+        const TBasicType requiredBasicType = useFloatClamp ? EbtFloat : EbtInt;
+        if (rightType.getBasicType() != requiredBasicType)
+        {
+            const TType *clampType =
+                useFloatClamp ? StaticType::GetBasic<EbtFloat>() : StaticType::GetBasic<EbtInt>();
+            TIntermSequence constructorArgs = {index};
+            index = TIntermAggregate::CreateConstructor(*clampType, &constructorArgs);
+        }
+
+        // min(gl_PointSize, maxPointSize)
+        TIntermSequence args;
+        args.push_back(index);
+        args.push_back(zero);
+        args.push_back(max);
+        TIntermTyped *clamped =
+            CreateBuiltInFunctionCallNode("clamp", &args, *mSymbolTable, useFloatClamp ? 100 : 300);
+
+        // Cast back to int if float clamp was used.
+        if (useFloatClamp)
+        {
+            TIntermSequence constructorArgs = {clamped};
+            clamped = TIntermAggregate::CreateConstructor(*StaticType::GetBasic<EbtInt>(),
+                                                          &constructorArgs);
+        }
+
+        // Replace the right node (the index) with the clamped result.
+        queueReplacementWithParent(node, node->getRight(), clamped, OriginalNode::IS_DROPPED);
+
+        // Don't recurse as left and right nodes are already processed.
+        return false;
+    }
+
+  private:
+    TIntermConstantUnion *createClampValue(int value, bool useFloat)
+    {
+        if (useFloat)
+        {
+            return CreateFloatNode(static_cast<float>(value));
+        }
+        return CreateIndexNode(value);
+    }
+
+    TCompiler *mCompiler;
+};
+}  // anonymous namespace
+
+bool ClampIndirectIndices(TCompiler *compiler, TIntermNode *root, TSymbolTable *symbolTable)
+{
+    ClampIndirectIndicesTraverser traverser(compiler, symbolTable);
+    root->traverse(&traverser);
+    return traverser.updateTree(compiler, root);
+}
+
+}  // namespace sh

src/compiler/translator/tree_ops/ClampIndirectIndices.h

+//
+// Copyright 2021 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// ClampIndirectIndices.h: Add clamp to the indirect indices used on arrays.
+//
+
+#ifndef COMPILER_TRANSLATOR_TREEOPS_CLAMPINDIRECTINDICES_H_
+#define COMPILER_TRANSLATOR_TREEOPS_CLAMPINDIRECTINDICES_H_
+
+#include "common/angleutils.h"
+
+namespace sh
+{
+
+class TCompiler;
+class TIntermNode;
+class TSymbolTable;
+
+ANGLE_NO_DISCARD bool ClampIndirectIndices(TCompiler *compiler,
+                                           TIntermNode *root,
+                                           TSymbolTable *symbolTable);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_TREEOPS_CLAMPINDIRECTINDICES_H_

src/tests/gl_tests/GLSLTest.cpp

@@ -8124,6 +8124,71 @@ foo
     ANGLE_GL_PROGRAM(program, kVS, kFS);
 }
 
+// Test that clamp applied on non-literal indices is correct on es 100 shaders.
+TEST_P(GLSLTest, ValidIndexClampES100)
+{
+    // http://anglebug.com/6027
+    ANGLE_SKIP_TEST_IF(IsD3D9());
+
+    constexpr char kFS[] = R"(
+precision mediump float;
+uniform int u;
+uniform mat4 m[2];
+void main()
+{
+    gl_FragColor = vec4(m[u][1].xyz, 1);
+}
+)";
+
+    ANGLE_GL_PROGRAM(program, essl1_shaders::vs::Simple(), kFS);
+    glUseProgram(program);
+
+    GLint uniformLocation = glGetUniformLocation(program, "u");
+    ASSERT_NE(-1, uniformLocation);
+
+    GLint matrixLocation = glGetUniformLocation(program, "m");
+    ASSERT_NE(matrixLocation, -1);
+    const std::array<GLfloat, 32> mValue = {{0.5f, 0.0f, 0.0f, 0.0f, 0.0f, 0.5f, 0.0f, 0.0f,
+                                             0.0f, 0.0f, 0.5f, 0.0f, 0.0f, 0.0f, 0.0f, 0.5f,
+                                             1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f, 0.0f, 0.0f,
+                                             0.0f, 0.0f, 1.0f, 0.0f, 0.0f, 0.0f, 0.0f, 1.0f}};
+    glUniformMatrix4fv(matrixLocation, 2, false, mValue.data());
+
+    glUniform1i(uniformLocation, 1);
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
+// Test that clamp applied on non-literal indices is correct on es 300 shaders.
+TEST_P(GLSLTest_ES3, ValidIndexClampES300)
+{
+    constexpr char kFS[] = R"(#version 300 es
+precision mediump float;
+out vec4 color;
+uniform int u;
+mat4 m[4] = mat4[4](mat4(0.25), mat4(0.5), mat4(1), mat4(0.75));
+void main()
+{
+    color = vec4(m[u][2].xyz, 1);
+}
+)";
+
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+    glUseProgram(program);
+
+    GLint uniformLocation = glGetUniformLocation(program, "u");
+    ASSERT_NE(-1, uniformLocation);
+
+    glUniform1i(uniformLocation, 2);
+    EXPECT_GL_NO_ERROR();
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+    EXPECT_GL_NO_ERROR();
+
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::blue);
+}
+
 // Tests constant folding of non-square 'matrixCompMult'.
 TEST_P(GLSLTest_ES3, NonSquareMatrixCompMult)
 {

src/third_party/compiler/ArrayBoundsClamper.cpp

-/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include "third_party/compiler/ArrayBoundsClamper.h"
-
-#include "compiler/translator/tree_util/IntermTraverse.h"
-
-// The built-in 'clamp' instruction only accepts floats and returns a float.  I
-// iterated a few times with our driver team who examined the output from our
-// compiler - they said the multiple casts generates more code than a single
-// function call.  An inline ternary operator might have been better, but since
-// the index value might be an expression itself, we'd have to make temporary
-// variables to avoid evaluating the expression multiple times.  And making
-// temporary variables was difficult because ANGLE would then need to make more
-// brutal changes to the expression tree.
-
-const char *kIntClampBegin = "// BEGIN: Generated code for array bounds clamping\n\n";
-const char *kIntClampEnd   = "// END: Generated code for array bounds clamping\n\n";
-const char *kIntClampDefinition =
-    "int webgl_int_clamp(int value, int minValue, int maxValue) { return ((value < minValue) ? "
-    "minValue : ((value > maxValue) ? maxValue : value)); }\n\n";
-
-namespace sh
-{
-
-namespace
-{
-
-class ArrayBoundsClamperMarker : public TIntermTraverser
-{
-  public:
-    ArrayBoundsClamperMarker() : TIntermTraverser(true, false, false), mNeedsClamp(false) {}
-
-    bool visitBinary(Visit visit, TIntermBinary *node) override
-    {
-        if (node->getOp() == EOpIndexIndirect)
-        {
-            TIntermTyped *left = node->getLeft();
-            if (left->isArray() || left->isVector() || left->isMatrix())
-            {
-                node->setAddIndexClamp();
-                mNeedsClamp = true;
-            }
-        }
-        return true;
-    }
-
-    bool GetNeedsClamp() { return mNeedsClamp; }
-
-  private:
-    bool mNeedsClamp;
-};
-
-}  // anonymous namespace
-
-ArrayBoundsClamper::ArrayBoundsClamper()
-    : mClampingStrategy(SH_CLAMP_WITH_CLAMP_INTRINSIC), mArrayBoundsClampDefinitionNeeded(false)
-{}
-
-void ArrayBoundsClamper::SetClampingStrategy(ShArrayIndexClampingStrategy clampingStrategy)
-{
-    mClampingStrategy = clampingStrategy;
-}
-
-void ArrayBoundsClamper::MarkIndirectArrayBoundsForClamping(TIntermNode *root)
-{
-    ASSERT(root);
-
-    ArrayBoundsClamperMarker clamper;
-    root->traverse(&clamper);
-    if (clamper.GetNeedsClamp())
-    {
-        SetArrayBoundsClampDefinitionNeeded();
-    }
-}
-
-void ArrayBoundsClamper::OutputClampingFunctionDefinition(TInfoSinkBase &out) const
-{
-    if (!mArrayBoundsClampDefinitionNeeded)
-    {
-        return;
-    }
-    if (mClampingStrategy != SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION)
-    {
-        return;
-    }
-    out << kIntClampBegin << kIntClampDefinition << kIntClampEnd;
-}
-
-}  // namespace sh

src/third_party/compiler/ArrayBoundsClamper.h

-/*
- * Copyright (C) 2012 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef THIRD_PARTY_COMPILER_ARRAYBOUNDSCLAMPER_H_
-#define THIRD_PARTY_COMPILER_ARRAYBOUNDSCLAMPER_H_
-
-#include "compiler/translator/InfoSink.h"
-#include "compiler/translator/IntermNode.h"
-
-namespace sh
-{
-
-class ArrayBoundsClamper
-{
-  public:
-    ArrayBoundsClamper();
-
-    // Must be set before compiling any shaders to ensure consistency
-    // between the translated shaders and any necessary prequel.
-    void SetClampingStrategy(ShArrayIndexClampingStrategy clampingStrategy);
-
-    // Marks nodes in the tree that index arrays indirectly as
-    // requiring clamping.
-    void MarkIndirectArrayBoundsForClamping(TIntermNode *root);
-
-    // If necessary, output array clamp function source into the shader source.
-    void OutputClampingFunctionDefinition(TInfoSinkBase &out) const;
-
-    void Cleanup() { mArrayBoundsClampDefinitionNeeded = false; }
-
-  private:
-    bool GetArrayBoundsClampDefinitionNeeded() const { return mArrayBoundsClampDefinitionNeeded; }
-    void SetArrayBoundsClampDefinitionNeeded() { mArrayBoundsClampDefinitionNeeded = true; }
-
-    ShArrayIndexClampingStrategy mClampingStrategy;
-    bool mArrayBoundsClampDefinitionNeeded;
-};
-
-}  // namespace sh
-
-#endif  // THIRD_PARTY_COMPILER_ARRAYBOUNDSCLAMPER_H_

src/third_party/compiler/LICENSE

-Copyright (C) 2012 Apple Inc. All rights reserved.
-
-Redistribution and use in source and binary forms, with or without
-modification, are permitted provided that the following conditions
-are met:
-1. Redistributions of source code must retain the above copyright
-   notice, this list of conditions and the following disclaimer.
-2. Redistributions in binary form must reproduce the above copyright
-   notice, this list of conditions and the following disclaimer in the
-   documentation and/or other materials provided with the distribution.
-
-THIS SOFTWARE IS PROVIDED BY APPLE, INC. ``AS IS'' AND ANY
-EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
-IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
-PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE, INC. OR
-CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
-EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
-PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
-PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
-OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

src/third_party/compiler/README.chromium

-Name: ANGLE array bounds clamper from WebKit
-Short Name: WebKit
-URL: http://webkit.org
-Version: 0
-License: BSD
-Security Critical: yes
-
-Description:
-Implements clamping of array indexing expressions during shader translation.
-
-Local Modifications:
-None