Incorporated patch from haixia@ changing the default implementation of

array index clamping to use the clamp intrinsic. This works more
reliably on pure OpenGL ES devices and on Windows.

Added a mechanism in ShBuiltInResources to choose the strategy for
array index clamping.

BUG=none
TEST=various out-of-bounds array indexing tests and various WebGL content
Review URL: https://codereview.appspot.com/7194051

git-svn-id: https://angleproject.googlecode.com/svn/branches/dx11proto@1798 736b8ea6-26fd-11df-bfd4-992fa37f6226

include/GLSLANG/ShaderLang.h

@@ -158,10 +158,21 @@ typedef enum {
   // 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.
+  // vec234, or mat234 type. The ShArrayIndexClampingStrategy enum,
+  // specified in the ShBuiltInResources when constructing the
+  // compiler, selects the strategy for the clamping implementation.
   SH_CLAMP_INDIRECT_ARRAY_BOUNDS = 0x1000
 } ShCompileOptions;
 
+// Defines alternate strategies for implementing array index clamping.
+typedef enum {
+  // 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
+} ShArrayIndexClampingStrategy;
+
 //
 // Driver must call this first, once, before doing any other
 // compiler operations.
@@ -204,6 +215,10 @@ typedef struct
     // Set a 64 bit hash function to enable user-defined name hashing.
     // Default is NULL.
     ShHashFunction64 HashFunction;
+
+    // Selects a strategy to use when implementing array index clamping.
+    // Default is SH_CLAMP_WITH_CLAMP_INTRINSIC.
+    ShArrayIndexClampingStrategy ArrayIndexClampingStrategy;
 } ShBuiltInResources;
 
 //

src/common/version.h

@@ -1,7 +1,7 @@
 #define MAJOR_VERSION 1
 #define MINOR_VERSION 1
 #define BUILD_VERSION 0
-#define BUILD_REVISION 1732
+#define BUILD_REVISION 1733
 
 #define STRINGIFY(x) #x
 #define MACRO_STRINGIFY(x) STRINGIFY(x)

src/compiler/Compiler.cpp

@@ -102,6 +102,7 @@ TShHandleBase::~TShHandleBase() {
 TCompiler::TCompiler(ShShaderType type, ShShaderSpec spec)
     : shaderType(type),
       shaderSpec(spec),
+      clampingStrategy(SH_CLAMP_WITH_CLAMP_INTRINSIC),
       builtInFunctionEmulator(type)
 {
     longNameMap = LongNameMap::GetInstance();
@@ -125,6 +126,9 @@ bool TCompiler::Init(const ShBuiltInResources& resources)
         return false;
     InitExtensionBehavior(resources, extensionBehavior);
 
+    arrayBoundsClamper.SetClampingStrategy(resources.ArrayIndexClampingStrategy);
+    clampingStrategy = resources.ArrayIndexClampingStrategy;
+
     hashFunction = resources.HashFunction;
 
     return true;
@@ -355,13 +359,17 @@ const TExtensionBehavior& TCompiler::getExtensionBehavior() const
     return extensionBehavior;
 }
 
-const BuiltInFunctionEmulator& TCompiler::getBuiltInFunctionEmulator() const
-{
-    return builtInFunctionEmulator;
-}
-
 const ArrayBoundsClamper& TCompiler::getArrayBoundsClamper() const
 {
     return arrayBoundsClamper;
 }
 
+ShArrayIndexClampingStrategy TCompiler::getArrayIndexClampingStrategy() const
+{
+    return clampingStrategy;
+}
+
+const BuiltInFunctionEmulator& TCompiler::getBuiltInFunctionEmulator() const
+{
+    return builtInFunctionEmulator;
+}

src/compiler/OutputESSL.cpp

@@ -7,10 +7,11 @@
 #include "compiler/OutputESSL.h"
 
 TOutputESSL::TOutputESSL(TInfoSinkBase& objSink,
+                         ShArrayIndexClampingStrategy clampingStrategy,
                          ShHashFunction64 hashFunction,
                          NameMap& nameMap,
                          TSymbolTable& symbolTable)
-    : TOutputGLSLBase(objSink, hashFunction, nameMap, symbolTable)
+    : TOutputGLSLBase(objSink, clampingStrategy, hashFunction, nameMap, symbolTable)
 {
 }
 

src/compiler/OutputESSL.h

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

src/compiler/OutputGLSL.cpp

@@ -7,10 +7,11 @@
 #include "compiler/OutputGLSL.h"
 
 TOutputGLSL::TOutputGLSL(TInfoSinkBase& objSink,
+                         ShArrayIndexClampingStrategy clampingStrategy,
                          ShHashFunction64 hashFunction,
                          NameMap& nameMap,
                          TSymbolTable& symbolTable)
-    : TOutputGLSLBase(objSink, hashFunction, nameMap, symbolTable)
+    : TOutputGLSLBase(objSink, clampingStrategy, hashFunction, nameMap, symbolTable)
 {
 }
 

src/compiler/OutputGLSL.h

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

src/compiler/OutputGLSLBase.cpp

@@ -40,12 +40,14 @@ bool isSingleStatement(TIntermNode* node) {
 }  // namespace
 
 TOutputGLSLBase::TOutputGLSLBase(TInfoSinkBase& objSink,
+                                 ShArrayIndexClampingStrategy clampingStrategy,
                                  ShHashFunction64 hashFunction,
                                  NameMap& nameMap,
                                  TSymbolTable& symbolTable)
     : TIntermTraverser(true, true, true),
       mObjSink(objSink),
       mDeclaringVariables(false),
+      mClampingStrategy(clampingStrategy),
       mHashFunction(hashFunction),
       mNameMap(nameMap),
       mSymbolTable(symbolTable)
@@ -221,7 +223,11 @@ bool TOutputGLSLBase::visitBinary(Visit visit, TIntermBinary* node)
             {
                 if (visit == InVisit)
                 {
-                    out << "[webgl_int_clamp(";
+                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC) {
+                        out << "[int(clamp(float(";
+                    } else {
+                        out << "[webgl_int_clamp(";
+                    }
                 }
                 else if (visit == PostVisit)
                 {
@@ -238,7 +244,12 @@ bool TOutputGLSLBase::visitBinary(Visit visit, TIntermBinary* node)
                     {
                         maxSize = leftType.getNominalSize() - 1;
                     }
-                    out << ", 0, " << maxSize << ")]";
+
+                    if (mClampingStrategy == SH_CLAMP_WITH_CLAMP_INTRINSIC) {
+                        out << "), 0.0, float(" << maxSize << ")))]";
+                    } else {
+                        out << ", 0, " << maxSize << ")]";
+                    }
                 }
             }
             else

src/compiler/OutputGLSLBase.h

@@ -17,6 +17,7 @@ class TOutputGLSLBase : public TIntermTraverser
 {
 public:
     TOutputGLSLBase(TInfoSinkBase& objSink,
+                    ShArrayIndexClampingStrategy clampingStrategy,
                     ShHashFunction64 hashFunction,
                     NameMap& nameMap,
                     TSymbolTable& symbolTable);
@@ -62,8 +63,11 @@ private:
 
     ForLoopUnroll mLoopUnroll;
 
+    ShArrayIndexClampingStrategy mClampingStrategy;
+
     // name hashing.
     ShHashFunction64 mHashFunction;
+
     NameMap& mNameMap;
 
     TSymbolTable& mSymbolTable;

src/compiler/ShHandle.h

@@ -110,6 +110,7 @@ protected:
     const TExtensionBehavior& getExtensionBehavior() const;
 
     const ArrayBoundsClamper& getArrayBoundsClamper() const;
+    ShArrayIndexClampingStrategy getArrayIndexClampingStrategy() const;
     const BuiltInFunctionEmulator& getBuiltInFunctionEmulator() const;
 
 private:
@@ -125,6 +126,7 @@ private:
     TExtensionBehavior extensionBehavior;
 
     ArrayBoundsClamper arrayBoundsClamper;
+    ShArrayIndexClampingStrategy clampingStrategy;
     BuiltInFunctionEmulator builtInFunctionEmulator;
 
     // Results of compilation.

src/compiler/ShaderLang.cpp

@@ -129,6 +129,8 @@ void ShInitBuiltInResources(ShBuiltInResources* resources)
 
     // Disable name hashing by default.
     resources->HashFunction = NULL;
+
+    resources->ArrayIndexClampingStrategy = SH_CLAMP_WITH_CLAMP_INTRINSIC;
 }
 
 //
@@ -363,4 +365,4 @@ void ShGetInfoPointer(const ShHandle handle, ShShaderInfo pname, void** params)
         break;
     default: UNREACHABLE();
     }
-}
+}

src/compiler/TranslatorESSL.cpp

@@ -26,7 +26,7 @@ void TranslatorESSL::translate(TIntermNode* root) {
     getArrayBoundsClamper().OutputClampingFunctionDefinition(sink);
 
     // Write translated shader.
-    TOutputESSL outputESSL(sink, getHashFunction(), getNameMap(), getSymbolTable());
+    TOutputESSL outputESSL(sink, getArrayIndexClampingStrategy(), getHashFunction(), getNameMap(), getSymbolTable());
     root->traverse(&outputESSL);
 }
 

src/compiler/TranslatorGLSL.cpp

@@ -39,6 +39,6 @@ void TranslatorGLSL::translate(TIntermNode* root) {
     getArrayBoundsClamper().OutputClampingFunctionDefinition(sink);
 
     // Write translated shader.
-    TOutputGLSL outputGLSL(sink, getHashFunction(), getNameMap(), getSymbolTable());
+    TOutputGLSL outputGLSL(sink, getArrayIndexClampingStrategy(), getHashFunction(), getNameMap(), getSymbolTable());
     root->traverse(&outputGLSL);
 }

src/third_party/compiler/ArrayBoundsClamper.cpp

@@ -70,17 +70,14 @@ private:
 }  // anonymous namespace
 
 ArrayBoundsClamper::ArrayBoundsClamper()
-    : mArrayBoundsClampDefinitionNeeded(false)
+    : mClampingStrategy(SH_CLAMP_WITH_CLAMP_INTRINSIC)
+    , mArrayBoundsClampDefinitionNeeded(false)
 {
 }
 
-void ArrayBoundsClamper::OutputClampingFunctionDefinition(TInfoSinkBase& out) const
+void ArrayBoundsClamper::SetClampingStrategy(ShArrayIndexClampingStrategy clampingStrategy)
 {
-    if (!mArrayBoundsClampDefinitionNeeded)
-    {
-        return;
-    }
-    out << kIntClampBegin << kIntClampDefinition << kIntClampEnd;
+    mClampingStrategy = clampingStrategy;
 }
 
 void ArrayBoundsClamper::MarkIndirectArrayBoundsForClamping(TIntermNode* root)
@@ -95,3 +92,15 @@ void ArrayBoundsClamper::MarkIndirectArrayBoundsForClamping(TIntermNode* root)
     }
 }
 
+void ArrayBoundsClamper::OutputClampingFunctionDefinition(TInfoSinkBase& out) const
+{
+    if (!mArrayBoundsClampDefinitionNeeded)
+    {
+        return;
+    }
+    if (mClampingStrategy != SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION)
+    {
+        return;
+    }
+    out << kIntClampBegin << kIntClampDefinition << kIntClampEnd;
+}

src/third_party/compiler/ArrayBoundsClamper.h

@@ -35,13 +35,17 @@ class ArrayBoundsClamper {
 public:
     ArrayBoundsClamper();
 
-    // Output array clamp function source into the shader source.
-    void OutputClampingFunctionDefinition(TInfoSinkBase& out) const;
+    // 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;
@@ -50,7 +54,8 @@ public:
 private:
     bool GetArrayBoundsClampDefinitionNeeded() const { return mArrayBoundsClampDefinitionNeeded; }
     void SetArrayBoundsClampDefinitionNeeded() { mArrayBoundsClampDefinitionNeeded = true; }
-    
+
+    ShArrayIndexClampingStrategy mClampingStrategy;
     bool mArrayBoundsClampDefinitionNeeded;
 };