Make unit tests more resilient before adding new rewrite (#366)

Co-authored-by: Laurent Le Brun <laurentlb@gmail.com>

src/analyzer.fs

@@ -8,6 +8,14 @@ open Options.Globals
 // information in the AST nodes, e.g. find which variables are modified,
 // which declarations can be inlined.
 
+let varUsesInStmt stmt = 
+    let mutable idents = []
+    let collectLocalUses _ = function
+        | Var v as e -> idents <- v :: idents; e
+        | e -> e
+    mapStmt BlockLevel.Unknown (mapEnvExpr collectLocalUses) stmt |> ignore<MapEnv * Stmt>
+    idents
+
 module private VariableInlining =
 
     // Return the list of variables used in the statements, with the number of references.
@@ -22,14 +30,6 @@ module private VariableInlining =
             mapStmt BlockLevel.Unknown (mapEnvExpr collectLocalUses) expr |> ignore<MapEnv * Stmt>
         counts
 
-    let collectReferencesSet expr  = // INCORRECT: the shadowing variables are merged! they might hide a writing reference!
-        let result = HashSet<Ident>()
-        let collectLocalUses _ = function
-            | Var v as e -> result.Add(v) |> ignore<bool>; e
-            | e -> e
-        mapExpr (mapEnvExpr collectLocalUses) expr |> ignore<Expr>
-        result
-
     let isEffectivelyConst (ident: Ident) =
         match ident.Declaration with
         | Declaration.Variable varDecl ->
@@ -59,8 +59,8 @@ module private VariableInlining =
                     | None -> ()
                     | Some init ->
                         localExpr <- init :: localExpr
-                        let deps = collectReferencesSet init
-                        let isConst = deps |> Seq.forall isEffectivelyConst
+                        let isConst = // INCORRECT: the shadowing variables are merged! they might hide a writing reference!
+                            varUsesInStmt (Expr init) |> Seq.forall isEffectivelyConst
                         localDefs.[def.name.Name] <- (def.name, isConst)
             | Expr e
             | Jump (_, Some e) -> localExpr <- e :: localExpr

src/rewriter.fs

@@ -416,6 +416,61 @@ module private RewriterImpl =
         | Switch (_e, cases) -> cases |> List.forall (fun (_label, stmts) -> hasNoContinue stmts)
         | _ -> true)
 
+    // Reuse an existing local variable declaration that won't be used anymore, instead of introducing a new one.
+    // The reused identifier gets compressed better, and the declaration is sometimes removed.
+    // float d1 = f(); float d2 = g();  ->  float d1 = f(); d1 = g();
+    let reuseExistingVarDecl b =
+        let tryReplaceWithPrecedingAndFollowing f (xs : Stmt list) =
+            let rec go f preceding = function
+                | head :: tail ->
+                    match f (preceding, head, tail) with
+                    | None -> head :: go f (head :: preceding) tail // preceding is reversed, that's good
+                    | Some xs -> xs @ tail
+                | [] -> []
+            go f [] xs
+        b |> tryReplaceWithPrecedingAndFollowing (function
+        | (preceding2, Decl (ty2, declElts), following2) ->
+            let findAssignmentReplacementFor declElt2 =
+                let compatibleDeclElts = preceding2 |> List.collect (function
+                    // The previous declaration must have the same type.
+                    | Decl (ty1, declElts1) when ty2 = ty1 ->
+                        let firstIsNotUsedAfterSecondDeclared (declElt1 : DeclElt) followingDecl2 =
+                            // The first variable must not be used after the second is declared.
+                            Analyzer.varUsesInStmt (Block followingDecl2) |> List.forall (fun i -> i.Name <> declElt1.name.Name)
+                        declElts1 |> List.filter (fun declElt1 ->
+                            // The previous declaration must have the same size and semantics
+                            declElt1.size = declElt2.size &&
+                            declElt1.semantics = declElt2.semantics &&
+                            firstIsNotUsedAfterSecondDeclared declElt1 following2
+                        )
+                    | _ -> [])
+                match compatibleDeclElts |> List.tryHead with
+                | Some (declElt1) ->
+                    debug $"{declElt2.name.Loc}: eliminating local variable '{declElt2.name}' by reusing existing local variable '{declElt1.name}'"
+                    for v in Analyzer.varUsesInStmt (Block following2) do // Rename all uses of var2 to use var1 instead.
+                        if v.Name = declElt2.name.Name then
+                            v.Rename(declElt1.name.Name)
+                    match declElt2.init with
+                    | Some init -> Some [Expr (FunCall (Op "=", [Var declElt1.name; init]))]
+                    | None -> Some []
+                | _ -> None
+            match declElts, (declElts |> List.rev) with
+            //| [declElt2], _  -> // float d1=f(); ...; float d2=g();  ->  ...; d1=g();
+            //    match findAssignmentReplacementFor declElt2 with
+            //    | Some stmts -> Some stmts // Replace the Decl with the assignment. Largest win
+            //    | None -> None
+            //| (declElt2 :: others), _ -> // float d1=f(); ...; float d2=g(),d3=h();  ->  ...; d1=g(); float d3=h();
+            //    match findAssignmentReplacementFor declElt2 with
+            //    | Some stmts -> Some (stmts @ [Decl (ty2, others)]) // Keep the Decl, add an assignment before it.
+            //    | None -> None
+            //| _, (declElt2 :: others) -> // float d1=f(); ...; float d3=h(),d2=g();  ->  ...; float d3=h(); d1=g();
+            //    match findAssignmentReplacementFor declElt2 with
+            //    | Some stmts -> Some (Decl (ty2, others) :: stmts) // Keep the Decl, add an assignment after it.
+            //    | None -> None
+            // For a decl sandwiched between others, we could consider moving the assignment into a comma-expr of the init of the next decl, but this adds parentheses.
+            | _ -> None
+        | _ -> None)
+
     let simplifyBlock blockLevel stmts = 
         let b = stmts
         // Avoid some optimizations when there are preprocessor directives.
@@ -522,6 +577,12 @@ module private RewriterImpl =
             | stmts -> stmts
         let b = replaceIfReturnsWithReturnTernary b
 
+        let b =
+            // We ensure control flow analysis is trivial by only doing this at the root block level.
+            if blockLevel <> BlockLevel.FunctionRoot || hasPreprocessor
+            then b
+            else reuseExistingVarDecl b
+
         // Consecutive declarations of the same type become one.  float a;float b;  ->  float a,b;
         let b = squeezeConsecutiveDeclarations b
 

tests/commands.txt

@@ -32,6 +32,7 @@
 --no-remove-unused --no-renaming --format indented -o tests/unit/precedence.frag.expected tests/unit/precedence.frag
 --no-remove-unused --format indented -o tests/unit/interface_block.frag.expected tests/unit/interface_block.frag
 --no-remove-unused --no-renaming --format indented -o tests/unit/ternary.frag.expected tests/unit/ternary.frag
+--no-remove-unused --no-renaming --format indented -o tests/unit/reuse-var.frag.expected tests/unit/reuse-var.frag
 
 # Unused removal tests
 

tests/unit/float.frag

@@ -1,18 +1,20 @@
-void floatPrecision() {
+float floatPrecision(float x) {
   float a = 0.00000012345;
   float b = 1.234567891234;
   float c = 1234567.891234;
   float d = 123456700000000.;
+  return a*x+b*x+c*x+d*x;
 }
 
-void largeNumbers() {
+float largeNumbers(float x) {
   float a = 4.1e8;
   float b = 4.2e10;
   float c = 4.3e12;
   float d = 4.4e14;
+  return a*x+b*x+c*x+d*x;
 }
 
-void smallNumbers() {
+float smallNumbers(float x) {
   float a = 4e-1;
   float b = 4e-2;
   float c = 4e-3;
@@ -21,14 +23,18 @@ void smallNumbers() {
   float f = 4e-10;
   float g = 4e-12;
   float h = 4e-14;
+  return a*x+b*x+c*x+d*x+e*x+f*x+g*x+h*x;
 }
 
-void zero() {
+float zero(float x) {
   float a = 0.;
   float b = .0;
   float c = -.0;
+  return a*x+b*x+c*x;
 }
 
+float x;
+
 void main()
 {
 float f1 = 1.5;
@@ -39,5 +45,5 @@ float f7 = 2E-9;
 float f8 = 2E+6;
 float f9 = 2e10;
 
-gl_FragColor=vec4(0.);
+gl_FragColor=vec4(f1*x+f2*x+f3*x+f6*x+f7*x+f8*x+f9*x);
 }

tests/unit/float.frag.expected

@@ -7,26 +7,31 @@
 #else // if SHADER_MINIFIER_IMPL
 
 // tests/unit/float.frag
- "void floatPrecision()"
+ "float floatPrecision(float x)"
  "{"
    "float a=1.2345e-7,b=1.234567891234,c=1234567.891234,d=1.234567e14;"
+   "return a*x+b*x+c*x+d*x;"
  "}"
- "void largeNumbers()"
+ "float largeNumbers(float x)"
  "{"
    "float a=4.1e8,b=4.2e10,c=4.3e12,d=4.4e14;"
+   "return a*x+b*x+c*x+d*x;"
  "}"
- "void smallNumbers()"
+ "float smallNumbers(float x)"
  "{"
    "float a=.4,b=.04,c=.004,d=4e-4,e=4e-8,f=4e-10,g=4e-12,h=4e-14;"
+   "return a*x+b*x+c*x+d*x+e*x+f*x+g*x+h*x;"
  "}"
- "void zero()"
+ "float zero(float x)"
  "{"
    "float a=0.,b=0.,c=0.;"
+   "return a*x+b*x+c*x;"
  "}"
+ "float x;"
  "void main()"
  "{"
    "float f1=1.5,f2=3e-7,f3=.42,f6=-.002,f7=2e-9,f8=2e6,f9=2e10;"
-   "gl_FragColor=vec4(0);"
+   "gl_FragColor=vec4(f1*x+f2*x+f3*x+f6*x+f7*x+f8*x+f9*x);"
  "}",
 
 #endif

tests/unit/operators.expected

@@ -8,10 +8,11 @@
 
 // tests/unit/operators.frag
  "#version 330\n"
- "void main()"
+ "float notmain()"
  "{"
    "int a=11,b=7,c=2;"
    "float f=1.4/3.,g=mod(8.,3.),z=122121;"
+   "return a+b+c+f+g+z;"
  "}"
  "int no_parens(int a,int b,int c)"
  "{"

tests/unit/operators.frag

@@ -1,6 +1,6 @@
 #version 330
 
-void main()
+float notmain()
 {
   int a = 1 + 2 * 3 + 4;
   int b = 14 / 2;
@@ -18,6 +18,8 @@ void main()
     (1. >= 2. ? 1 : 2) * 1000 +
 	(1. == 2. ? 1 : 2) * 10000 +
 	(1. != 2. ? 1 : 2) * 100000;
+
+  return a+b+c+f+g+z;
 }
 
 int no_parens(int a, int b, int c) {

tests/unit/pi.frag

@@ -1,5 +1,7 @@
 #version 400
 
+out vec4 fragColor;
+
 void main() {
     double pi = 3.141592653589793;
     double minus_pi = -3.141592653589793;
@@ -8,4 +10,5 @@ void main() {
     double half_pi = 1.5707963267948966;
     double minus_half_pi = -1.5707963267948966;
     double precise_pi = 3.14159265358979323846264338327950288419716939937510;
+	fragColor = vec4(pi+minus_pi+tau+minus_tau+half_pi+minus_half_pi+precise_pi);
 }

tests/unit/pi.frag.expected

@@ -1,6 +1,8 @@
 #version 400
 
+out vec4 fragColor;
 void main()
 {
   double pi=acos(-1.),minus_pi=-acos(-1.),tau=2.*acos(-1.),minus_tau=-2.*acos(-1.),half_pi=acos(0.),minus_half_pi=-acos(0.),precise_pi=acos(-1.);
+  fragColor=vec4(pi+minus_pi+tau+minus_tau+half_pi+minus_half_pi+precise_pi);
 }

tests/unit/reuse-var.frag

@@ -0,0 +1,37 @@
+float simple_var_decl_reuse(float x)
+{
+	float a = 1.+x;
+	int b = 3+int(x);
+	vec3 sep = vec3(0.); // prevents squeezeConsecutiveDeclarations
+	int b2 = 5+int(a);
+	float c = a;
+	sep += vec3(0.); // prevents squeezeConsecutiveDeclarations
+	float c2 = 9.+a;
+	int d = 3+b+b2;
+	sep += vec3(0.); // prevents squeezeConsecutiveDeclarations
+	int d2 = d+b2-b;
+	float e = c*c2;
+	sep += vec3(0.); // prevents squeezeConsecutiveDeclarations
+	float e2 = 4.-c-c2;
+	int f = 3*d*d2;
+	sep += vec3(0.); // prevents squeezeConsecutiveDeclarations
+	int f2 = 4/d2-d;
+	float g = e-float(f)+e2;
+	int g2 = int(4.-g+e2+e);
+	int h = 3*f-f2;
+	sep += vec3(0.); // prevents squeezeConsecutiveDeclarations
+	int h2 = 7*f2-f;
+	return length(sep)+float(h*h2)*g*float(g2)*x + float(g2/h2-h);
+}
+float multidecl_var_decl_reuse(float x)
+{
+	float a = 1.+x;
+	int b = 3+int(x), b2 = 5+int(a);
+	float c = a, c2 = 9.+a;
+	int d = 3+b+b2, d2 = d+b2-b;
+	float e = c*c2, e2 = 4.-c-c2;
+	int f = 3*d*d2, f2 = 4/d2-d;
+	float g = e-float(f)+e2, g2 = 4.-g+e2+e;
+	int h = 3*f-f2, h2 = 7*f2-f;
+	return float(h*h2)*g*g2*x + g2/float(h2-h);
+}

tests/unit/reuse-var.frag.expected

@@ -0,0 +1,36 @@
+float simple_var_decl_reuse(float x)
+{
+  float a=1.+x;
+  int b=3+int(x);
+  vec3 sep=vec3(0);
+  int b2=5+int(a);
+  float c=a;
+  sep+=vec3(0);
+  float c2=9.+a;
+  int d=3+b+b2;
+  sep+=vec3(0);
+  int d2=d+b2-b;
+  float e=c*c2;
+  sep+=vec3(0);
+  float e2=4.-c-c2;
+  int f=3*d*d2;
+  sep+=vec3(0);
+  int f2=4/d2-d;
+  float g=e-float(f)+e2;
+  int g2=int(4.-g+e2+e),h=3*f-f2;
+  sep+=vec3(0);
+  int h2=7*f2-f;
+  return length(sep)+float(h*h2)*g*float(g2)*x+float(g2/h2-h);
+}
+float multidecl_var_decl_reuse(float x)
+{
+  float a=1.+x;
+  int b=3+int(x),b2=5+int(a);
+  float c=a,c2=9.+a;
+  int d=3+b+b2,d2=d+b2-b;
+  float e=c*c2,e2=4.-c-c2;
+  int f=3*d*d2,f2=4/d2-d;
+  float g=e-float(f)+e2,g2=4.-g+e2+e;
+  int h=3*f-f2,h2=7*f2-f;
+  return float(h*h2)*g*g2*x+g2/float(h2-h);
+}