Better reduction for array indices

Improves the reducer's ability to reduce array indexing expressions,
by being less conservative about l-values.

Part of #1179.

Author: afd

reducer/src/main/java/com/graphicsfuzz/reducer/reductionopportunities/CompoundExprToSubExprReductionOpportunities.java

@@ -18,6 +18,7 @@
 
 import com.graphicsfuzz.common.ast.IAstNode;
 import com.graphicsfuzz.common.ast.TranslationUnit;
+import com.graphicsfuzz.common.ast.expr.ArrayIndexExpr;
 import com.graphicsfuzz.common.ast.expr.BinOp;
 import com.graphicsfuzz.common.ast.expr.BinaryExpr;
 import com.graphicsfuzz.common.ast.expr.ConstantExpr;
@@ -51,7 +52,7 @@ void identifyReductionOpportunitiesForChild(IAstNode parent, Expr child) {
     if (child instanceof ParenExpr) {
       // We need to be careful about removing parentheses, as this can change precedence.
       // We only remove parentheses if they are at the root of an expression, immediately under
-      // other parentheses, or immediately under a function call.
+      // other parentheses, immediately under a function call, or immediately under array brackets.
       if (!isOkToRemoveParens(parent, (ParenExpr) child)) {
         return;
       }
@@ -97,6 +98,10 @@ private boolean isOkToRemoveParens(IAstNode parent, ParenExpr child) {
       // These are parentheses within parentheses; fine to remove them.
       return true;
     }
+    if (parent instanceof ArrayIndexExpr && child == ((ArrayIndexExpr)parent).getIndex()) {
+      // These are parenthesis directly under array indexing brackets; fine to remove them.
+      return true;
+    }
     if (parent instanceof FunctionCallExpr) {
       // These are parentheses under a function call argument.  Fine to remove them *unless*
       // they enclose a use of the comma operator; e.g. we don't want to turn sin((a, b)) into

reducer/src/main/java/com/graphicsfuzz/reducer/reductionopportunities/ReductionOpportunitiesBase.java

@@ -19,6 +19,7 @@
 import com.graphicsfuzz.common.ast.IAstNode;
 import com.graphicsfuzz.common.ast.TranslationUnit;
 import com.graphicsfuzz.common.ast.decl.VariableDeclInfo;
+import com.graphicsfuzz.common.ast.expr.ArrayIndexExpr;
 import com.graphicsfuzz.common.ast.expr.BinaryExpr;
 import com.graphicsfuzz.common.ast.expr.Expr;
 import com.graphicsfuzz.common.ast.expr.UnaryExpr;
@@ -38,7 +39,11 @@ public abstract class ReductionOpportunitiesBase
 
   final ShaderKind shaderKind;
 
-  private int numEnclosingLValues;
+  // Each entry in this stack records how many l-values enclose the current expression. A new
+  // stack entry is pushed each time an array indexing expression is encountered, because even when
+  // an array indexing expression is an l-value, this does not mean that the index expression
+  // itself is an l-value.
+  private final List<Integer> enclosingLValuesStack;
 
   /**
    * Construct base class for finding reduction opportunities, with respect to a translation unit.
@@ -51,7 +56,8 @@ public ReductionOpportunitiesBase(TranslationUnit tu, ReducerContext context) {
     this.opportunities = new ArrayList<>();
     this.context = context;
     this.shaderKind = tu.getShaderKind();
-    this.numEnclosingLValues = 0;
+    this.enclosingLValuesStack = new ArrayList<>();
+    this.enclosingLValuesStack.add(0);
   }
 
   @Override
@@ -86,6 +92,13 @@ public void visitUnaryExpr(UnaryExpr unaryExpr) {
     }
   }
 
+  @Override
+  public void visitArrayIndexExpr(ArrayIndexExpr arrayIndexExpr) {
+    enclosingLValuesStack.add(0);
+    super.visitArrayIndexExpr(arrayIndexExpr);
+    enclosingLValuesStack.remove(enclosingLValuesStack.size() - 1);
+  }
+
   @Override
   protected void visitChildFromParent(IAstNode child, IAstNode parent) {
     super.visitChildFromParent(child, parent);
@@ -99,16 +112,18 @@ void identifyReductionOpportunitiesForChild(IAstNode parent, Expr child) {
   }
 
   private void enterLValueContext() {
-    numEnclosingLValues++;
+    enclosingLValuesStack.set(enclosingLValuesStack.size() - 1,
+        enclosingLValuesStack.get(enclosingLValuesStack.size() - 1) + 1);
   }
 
   private void exitLValueContext() {
-    assert numEnclosingLValues > 0;
-    numEnclosingLValues--;
+    assert enclosingLValuesStack.get(enclosingLValuesStack.size() - 1) > 0;
+    enclosingLValuesStack.set(enclosingLValuesStack.size() - 1,
+        enclosingLValuesStack.get(enclosingLValuesStack.size() - 1) - 1);
   }
 
   boolean inLValueContext() {
-    return numEnclosingLValues > 0;
+    return enclosingLValuesStack.get(enclosingLValuesStack.size() - 1) > 0;
   }
 
   boolean initializerIsScalarAndSideEffectFree(VariableDeclInfo variableDeclInfo) {

reducer/src/test/java/com/graphicsfuzz/reducer/ReductionDriverTest.java

@@ -1064,4 +1064,60 @@ public boolean isInteresting(
         resultsPrefix + ".frag")));
   }
 
+  @Test
+  public void testSimplificationOfTernaryWithArray() throws Exception {
+    final String shader = "#version 310 es\n"
+        + "void main()\n"
+        + "{\n"
+        + " int ext_0;\n"
+        + " int ext_1[3];\n"
+        + " int ext_2[3];\n"
+        + " ext_2[(all(bvec2(true)) ? (+ abs(ext_1[1])) : (ext_0 & -- ext_2[2]))] |= 1;\n"
+        + "}\n";
+
+    final String expected = "#version 310 es\n"
+        + "void main()\n"
+        + "{\n"
+        + " int ext_1[3];\n"
+        + " int ext_2[3];\n"
+        + " int[3](1, 1, 1)[abs(ext_1[1])] |= 1;\n"
+        + "}\n";
+
+    final ShaderJob shaderJob = new GlslShaderJob(Optional.empty(),
+        new PipelineInfo(),
+        ParseHelper.parse(shader));
+
+    final File workDir = testFolder.getRoot();
+    final File tempShaderJobFile = new File(workDir, "temp.json");
+    fileOps.writeShaderJobFile(shaderJob, tempShaderJobFile);
+
+    final IFileJudge customJudge = (file, unused) -> {
+      try {
+        ShaderJob customJudgeShaderJob = fileOps.readShaderJobFile(file);
+        assert customJudgeShaderJob.getShaders().size() == 1;
+        final TranslationUnit tu = customJudgeShaderJob.getShaders().get(0);
+        final String shaderString = tu.getText();
+        return shaderString.contains("ext_2[") && shaderString.contains("abs(ext_1[1])")
+            && shaderString.contains(" |= 1");
+      } catch (Exception exception) {
+        throw new RuntimeException(exception);
+      }
+    };
+
+    final String resultsPrefix = new ReductionDriver(new ReducerContext(true,
+        false,
+        ShadingLanguageVersion.ESSL_310,
+        new RandomWrapper(0),
+        new IdGenerator()),
+        false,
+        fileOps,
+        customJudge,
+        workDir)
+        .doReduction(shaderJob, "temp", 0, 100);
+
+    CompareAsts.assertEqualAsts(expected, ParseHelper.parse(new File(testFolder.getRoot(),
+        resultsPrefix + ".frag")));
+
+  }
+
 }

reducer/src/test/java/com/graphicsfuzz/reducer/reductionopportunities/CompoundExprToSubExprReductionOpportunitiesTest.java

@@ -22,6 +22,7 @@
 import com.graphicsfuzz.common.ast.TranslationUnit;
 import com.graphicsfuzz.common.glslversion.ShadingLanguageVersion;
 import com.graphicsfuzz.common.tool.PrettyPrinterVisitor;
+import com.graphicsfuzz.common.util.CompareAsts;
 import com.graphicsfuzz.common.util.GlslParserException;
 import com.graphicsfuzz.common.util.IdGenerator;
 import com.graphicsfuzz.common.util.ParseHelper;
@@ -191,4 +192,55 @@ public void testSwitch() throws Exception {
     assertEquals(0, ops.size());
   }
 
+  @Test
+  public void testTernary() throws Exception {
+    final String program = "#version 310 es\n"
+        + "void main() {\n"
+        + "  true ? 1 : 0;\n"
+        + "}\n";
+    final TranslationUnit tu = ParseHelper.parse(program);
+    final List<SimplifyExprReductionOpportunity> ops = CompoundExprToSubExprReductionOpportunities
+        .findOpportunities(MakeShaderJobFromFragmentShader.make(tu),
+            new ReducerContext(true, true, ShadingLanguageVersion.ESSL_310, new RandomWrapper(0),
+                new IdGenerator()));
+    assertEquals(2, ops.size());
+    ops.get(0).applyReduction();
+    final String expected = "#version 310 es\n"
+        + "void main() {\n"
+        + "  1;\n"
+        + "}\n";
+    CompareAsts.assertEqualAsts(expected, tu);
+  }
+
+  @Test
+  public void testArrayIndexWithParentheses() throws Exception {
+    final String program = "#version 310 es\n"
+        + "void main() {\n"
+        + "  int x, y;"
+        + "  int A[1];\n"
+        + "  A[(x + y)];\n"
+        + "}\n";
+    final TranslationUnit tu = ParseHelper.parse(program);
+    final List<SimplifyExprReductionOpportunity> ops = CompoundExprToSubExprReductionOpportunities
+        .findOpportunities(MakeShaderJobFromFragmentShader.make(tu),
+            new ReducerContext(true, true, ShadingLanguageVersion.ESSL_310, new RandomWrapper(0),
+                new IdGenerator()));
+    assertEquals(4, ops.size());
+  }
+
+  @Test
+  public void testSimplifyArrayIndexOfLValue() throws Exception {
+    final String program = "#version 310 es\n"
+        + "void main() {\n"
+        + "  int A[1];\n"
+        + "  A[(0)] = 1;\n"
+        + "}\n";
+    final TranslationUnit tu = ParseHelper.parse(program);
+    final List<SimplifyExprReductionOpportunity> ops = CompoundExprToSubExprReductionOpportunities
+        .findOpportunities(MakeShaderJobFromFragmentShader.make(tu),
+            new ReducerContext(true, true, ShadingLanguageVersion.ESSL_310, new RandomWrapper(0),
+                new IdGenerator()));
+    assertEquals(3, ops.size());
+  }
+
 }