Add SH_REMOVE_DYNAMIC_INDEXING_OF_SWIZZLED_VECTOR

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 217
+#define ANGLE_SH_VERSION 218
 
 enum ShShaderSpec
 {
@@ -308,6 +308,9 @@ const ShCompileOptions SH_USE_OLD_REWRITE_STRUCT_SAMPLERS = UINT64_C(1) << 47;
 // when angle_BaseVertex is available.
 const ShCompileOptions SH_ADD_BASE_VERTEX_TO_VERTEX_ID = UINT64_C(1) << 48;
 
+// This works around the dynamic lvalue indexing of swizzled vectors on various platforms.
+const ShCompileOptions SH_REMOVE_DYNAMIC_INDEXING_OF_SWIZZLED_VECTOR = UINT64_C(1) << 49;
+
 // Defines alternate strategies for implementing array index clamping.
 enum ShArrayIndexClampingStrategy
 {

include/platform/FeaturesGL.h

@@ -361,6 +361,12 @@ struct FeaturesGL : FeatureSetBase
         "When GL_PRIMITIVE_RESTART_FIXED_INDEX is not available, emulate it with "
         "GL_PRIMITIVE_RESTART and glPrimitiveRestartIndex.",
         &members, "http://anglebug.com/3997"};
+
+    // Dynamic indexing of swizzled l-values doesn't work correctly on various platforms.
+    Feature removeDynamicIndexingOfSwizzledVector = {
+        "remove_dynamic_indexing_of_swizzled_vector", FeatureCategory::OpenGLWorkarounds,
+        "Dynamic indexing of swizzled l-values doesn't work correctly on various platforms.",
+        &members, "http://crbug.com/709351"};
 };
 
 inline FeaturesGL::FeaturesGL()  = default;

src/compiler.gni

@@ -152,6 +152,8 @@ angle_translator_sources = [
   "src/compiler/translator/tree_ops/RegenerateStructNames.h",
   "src/compiler/translator/tree_ops/RemoveArrayLengthMethod.cpp",
   "src/compiler/translator/tree_ops/RemoveArrayLengthMethod.h",
+  "src/compiler/translator/tree_ops/RemoveDynamicIndexing.cpp",
+  "src/compiler/translator/tree_ops/RemoveDynamicIndexing.h",
   "src/compiler/translator/tree_ops/RemoveInvariantDeclaration.cpp",
   "src/compiler/translator/tree_ops/RemoveInvariantDeclaration.h",
   "src/compiler/translator/tree_ops/RemovePow.cpp",
@@ -276,8 +278,6 @@ angle_translator_hlsl_sources = [
   "src/compiler/translator/tree_ops/AddDefaultReturnStatements.h",
   "src/compiler/translator/tree_ops/ArrayReturnValueToOutParameter.cpp",
   "src/compiler/translator/tree_ops/ArrayReturnValueToOutParameter.h",
-  "src/compiler/translator/tree_ops/RemoveDynamicIndexing.cpp",
-  "src/compiler/translator/tree_ops/RemoveDynamicIndexing.h",
   "src/compiler/translator/tree_ops/RemoveSwitchFallThrough.cpp",
   "src/compiler/translator/tree_ops/RemoveSwitchFallThrough.h",
   "src/compiler/translator/tree_ops/RewriteElseBlocks.cpp",

src/compiler/translator/Compiler.cpp

@@ -35,6 +35,7 @@
 #include "compiler/translator/tree_ops/PruneNoOps.h"
 #include "compiler/translator/tree_ops/RegenerateStructNames.h"
 #include "compiler/translator/tree_ops/RemoveArrayLengthMethod.h"
+#include "compiler/translator/tree_ops/RemoveDynamicIndexing.h"
 #include "compiler/translator/tree_ops/RemoveInvariantDeclaration.h"
 #include "compiler/translator/tree_ops/RemovePow.h"
 #include "compiler/translator/tree_ops/RemoveUnreferencedVariables.h"
@@ -893,6 +894,14 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
         }
     }
 
+    if (compileOptions & SH_REMOVE_DYNAMIC_INDEXING_OF_SWIZZLED_VECTOR)
+    {
+        if (!sh::RemoveDynamicIndexingOfSwizzledVector(this, root, &getSymbolTable(), nullptr))
+        {
+            return false;
+        }
+    }
+
     return true;
 }
 

src/compiler/translator/TranslatorHLSL.cpp

@@ -105,7 +105,8 @@ bool TranslatorHLSL::translate(TIntermBlock *root,
     if (!shouldRunLoopAndIndexingValidation(compileOptions))
     {
         // HLSL doesn't support dynamic indexing of vectors and matrices.
-        if (!RemoveDynamicIndexing(this, root, &getSymbolTable(), perfDiagnostics))
+        if (!RemoveDynamicIndexingOfNonSSBOVectorOrMatrix(this, root, &getSymbolTable(),
+                                                          perfDiagnostics))
         {
             return false;
         }

src/compiler/translator/tree_ops/RemoveDynamicIndexing.cpp

@@ -26,6 +26,8 @@ namespace sh
 namespace
 {
 
+using DynamicIndexingNodeMatcher = std::function<bool(TIntermBinary *)>;
+
 const TType *kIndexType = StaticType::Get<EbtInt, EbpHigh, EvqIn, 1, 1>();
 
 constexpr const ImmutableString kBaseName("base");
@@ -261,7 +263,8 @@ TIntermFunctionDefinition *GetIndexFunctionDefinition(const TType &type,
 class RemoveDynamicIndexingTraverser : public TLValueTrackingTraverser
 {
   public:
-    RemoveDynamicIndexingTraverser(TSymbolTable *symbolTable,
+    RemoveDynamicIndexingTraverser(DynamicIndexingNodeMatcher &&matcher,
+                                   TSymbolTable *symbolTable,
                                    PerformanceDiagnostics *perfDiagnostics);
 
     bool visitBinary(Visit visit, TIntermBinary *node) override;
@@ -287,15 +290,18 @@ class RemoveDynamicIndexingTraverser : public TLValueTrackingTraverser
     // where V is an array of vectors, j++ will only be evaluated once.
     bool mRemoveIndexSideEffectsInSubtree;
 
+    DynamicIndexingNodeMatcher mMatcher;
     PerformanceDiagnostics *mPerfDiagnostics;
 };
 
 RemoveDynamicIndexingTraverser::RemoveDynamicIndexingTraverser(
+    DynamicIndexingNodeMatcher &&matcher,
     TSymbolTable *symbolTable,
     PerformanceDiagnostics *perfDiagnostics)
     : TLValueTrackingTraverser(true, false, false, symbolTable),
       mUsedTreeInsertion(false),
       mRemoveIndexSideEffectsInSubtree(false),
+      mMatcher(matcher),
       mPerfDiagnostics(perfDiagnostics)
 {}
 
@@ -376,12 +382,15 @@ bool RemoveDynamicIndexingTraverser::visitBinary(Visit visit, TIntermBinary *nod
             TIntermSymbol *tempIndex = CreateTempSymbolNode(indexVariable);
             queueReplacementWithParent(node, node->getRight(), tempIndex, OriginalNode::IS_DROPPED);
         }
-        else if (IntermNodePatternMatcher::IsDynamicIndexingOfNonSSBOVectorOrMatrix(node))
+        else if (mMatcher(node))
         {
-            mPerfDiagnostics->warning(node->getLine(),
-                                      "Performance: dynamic indexing of vectors and "
-                                      "matrices is emulated and can be slow.",
-                                      "[]");
+            if (mPerfDiagnostics)
+            {
+                mPerfDiagnostics->warning(node->getLine(),
+                                          "Performance: dynamic indexing of vectors and "
+                                          "matrices is emulated and can be slow.",
+                                          "[]");
+            }
             bool write = isLValueRequiredHere();
 
 #if defined(ANGLE_ENABLE_ASSERTS)
@@ -430,8 +439,7 @@ bool RemoveDynamicIndexingTraverser::visitBinary(Visit visit, TIntermBinary *nod
                 }
 
                 TIntermBinary *leftBinary = node->getLeft()->getAsBinaryNode();
-                if (leftBinary != nullptr &&
-                    IntermNodePatternMatcher::IsDynamicIndexingOfNonSSBOVectorOrMatrix(leftBinary))
+                if (leftBinary != nullptr && mMatcher(leftBinary))
                 {
                     // This is a case like:
                     // mat2 m;
@@ -520,14 +528,13 @@ void RemoveDynamicIndexingTraverser::nextIteration()
     mRemoveIndexSideEffectsInSubtree = false;
 }
 
-}  // namespace
-
-bool RemoveDynamicIndexing(TCompiler *compiler,
-                           TIntermNode *root,
-                           TSymbolTable *symbolTable,
-                           PerformanceDiagnostics *perfDiagnostics)
+bool RemoveDynamicIndexingIf(DynamicIndexingNodeMatcher &&matcher,
+                             TCompiler *compiler,
+                             TIntermNode *root,
+                             TSymbolTable *symbolTable,
+                             PerformanceDiagnostics *perfDiagnostics)
 {
-    RemoveDynamicIndexingTraverser traverser(symbolTable, perfDiagnostics);
+    RemoveDynamicIndexingTraverser traverser(std::move(matcher), symbolTable, perfDiagnostics);
     do
     {
         traverser.nextIteration();
@@ -546,4 +553,31 @@ bool RemoveDynamicIndexing(TCompiler *compiler,
     return compiler->validateAST(root);
 }
 
+}  // namespace
+
+ANGLE_NO_DISCARD bool RemoveDynamicIndexingOfNonSSBOVectorOrMatrix(
+    TCompiler *compiler,
+    TIntermNode *root,
+    TSymbolTable *symbolTable,
+    PerformanceDiagnostics *perfDiagnostics)
+{
+    DynamicIndexingNodeMatcher matcher = [](TIntermBinary *node) {
+        return IntermNodePatternMatcher::IsDynamicIndexingOfNonSSBOVectorOrMatrix(node);
+    };
+    return RemoveDynamicIndexingIf(std::move(matcher), compiler, root, symbolTable,
+                                   perfDiagnostics);
+}
+
+ANGLE_NO_DISCARD bool RemoveDynamicIndexingOfSwizzledVector(TCompiler *compiler,
+                                                            TIntermNode *root,
+                                                            TSymbolTable *symbolTable,
+                                                            PerformanceDiagnostics *perfDiagnostics)
+{
+    DynamicIndexingNodeMatcher matcher = [](TIntermBinary *node) {
+        return IntermNodePatternMatcher::IsDynamicIndexingOfSwizzledVector(node);
+    };
+    return RemoveDynamicIndexingIf(std::move(matcher), compiler, root, symbolTable,
+                                   perfDiagnostics);
+}
+
 }  // namespace sh

src/compiler/translator/tree_ops/RemoveDynamicIndexing.h

@@ -14,18 +14,28 @@
 
 #include "common/angleutils.h"
 
+#include <functional>
+
 namespace sh
 {
 
 class TCompiler;
 class TIntermNode;
+class TIntermBinary;
 class TSymbolTable;
 class PerformanceDiagnostics;
 
-ANGLE_NO_DISCARD bool RemoveDynamicIndexing(TCompiler *compiler,
-                                            TIntermNode *root,
-                                            TSymbolTable *symbolTable,
-                                            PerformanceDiagnostics *perfDiagnostics);
+ANGLE_NO_DISCARD bool RemoveDynamicIndexingOfNonSSBOVectorOrMatrix(
+    TCompiler *compiler,
+    TIntermNode *root,
+    TSymbolTable *symbolTable,
+    PerformanceDiagnostics *perfDiagnostics);
+
+ANGLE_NO_DISCARD bool RemoveDynamicIndexingOfSwizzledVector(
+    TCompiler *compiler,
+    TIntermNode *root,
+    TSymbolTable *symbolTable,
+    PerformanceDiagnostics *perfDiagnostics);
 
 }  // namespace sh
 

src/compiler/translator/tree_util/IntermNodePatternMatcher.cpp

@@ -59,6 +59,12 @@ bool IntermNodePatternMatcher::IsDynamicIndexingOfVectorOrMatrix(TIntermBinary *
            node->getLeft()->getBasicType() != EbtStruct;
 }
 
+// static
+bool IntermNodePatternMatcher::IsDynamicIndexingOfSwizzledVector(TIntermBinary *node)
+{
+    return IsDynamicIndexingOfVectorOrMatrix(node) && node->getLeft()->getAsSwizzleNode();
+}
+
 bool IntermNodePatternMatcher::matchInternal(TIntermBinary *node, TIntermNode *parentNode)
 {
     if ((mMask & kExpressionReturningArray) != 0)

src/compiler/translator/tree_util/IntermNodePatternMatcher.h

@@ -26,6 +26,7 @@ class IntermNodePatternMatcher
   public:
     static bool IsDynamicIndexingOfNonSSBOVectorOrMatrix(TIntermBinary *node);
     static bool IsDynamicIndexingOfVectorOrMatrix(TIntermBinary *node);
+    static bool IsDynamicIndexingOfSwizzledVector(TIntermBinary *node);
 
     enum PatternType
     {

src/libANGLE/renderer/gl/ShaderGL.cpp

@@ -343,6 +343,11 @@ std::shared_ptr<WaitableCompileEvent> ShaderGL::compile(const gl::Context *conte
         additionalOptions |= SH_UNFOLD_SHORT_CIRCUIT;
     }
 
+    if (features.removeDynamicIndexingOfSwizzledVector.enabled)
+    {
+        additionalOptions |= SH_REMOVE_DYNAMIC_INDEXING_OF_SWIZZLED_VECTOR;
+    }
+
     options |= additionalOptions;
 
     auto workerThreadPool = context->getWorkerThreadPool();

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1578,6 +1578,9 @@ void InitializeFeatures(const FunctionsGL *functions, angle::FeaturesGL *feature
     ANGLE_FEATURE_CONDITION(
         features, emulatePrimitiveRestartFixedIndex,
         !functions->isAtLeastGL(gl::Version(4, 3)) && !functions->isAtLeastGLES(gl::Version(3, 0)));
+
+    ANGLE_FEATURE_CONDITION(features, removeDynamicIndexingOfSwizzledVector,
+                            IsApple() || IsAndroid() || IsWindows());
 }
 
 void InitializeFrontendFeatures(const FunctionsGL *functions, angle::FrontendFeatures *features)

src/tests/gl_tests/GLSLTest.cpp

@@ -7266,6 +7266,30 @@ void main()
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Test that dynamic indexing of swizzled l-values should work.
+// A simple porting of sdk/tests/conformance2/glsl3/vector-dynamic-indexing-swizzled-lvalue.html
+TEST_P(GLSLTest_ES3, DynamicIndexingOfSwizzledLValuesShouldWork)
+{
+    // The shader first assigns v.x to v.z (1.0)
+    // Then v.y to v.y (2.0)
+    // Then v.z to v.x (1.0)
+    constexpr char kFS[] = R"(#version 300 es
+precision highp float;
+out vec4 my_FragColor;
+void main() {
+    vec3 v = vec3(1.0, 2.0, 3.0);
+    for (int i = 0; i < 3; i++) {
+        v.zyx[i] = v[i];
+    }
+    my_FragColor = distance(v, vec3(1.0, 2.0, 1.0)) < 0.01 ? vec4(0, 1, 0, 1) : vec4(1, 0, 0, 1);
+})";
+
+    ANGLE_GL_PROGRAM(program, essl3_shaders::vs::Simple(), kFS);
+    EXPECT_GL_NO_ERROR();
+    drawQuad(program, essl3_shaders::PositionAttrib(), 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Test that multiple nested assignments are handled correctly.
 TEST_P(GLSLTest_ES31, MixedRowAndColumnMajorMatrices_WriteSideEffect)
 {