Borked loop merging and/or common subexpression elimination

[miklos1]

Same test case as in #98, but not generating spurious empty loops per firedrakeproject/tsfc@509cb75, as you can see here. This time, however, the test fails because the result is numerically wrong. Looking at the change COFFEE does explains why:

--- coffee.c    2016-10-25 12:15:47.165410931 +0100
+++ coffee.c    2016-10-25 12:15:57.533559287 +0100
@@ -57,15 +57,10 @@
   for (int  ip_1  = 0; ip_1 < 4; ip_1 += 1)
   {

-    for (int  i_0  = 0; i_0 < 10; i_0 += 1)
-    {
-      t6[ip_1][i_0] = 0.0;
-      
-    }
-    
     for (int  i_1  = 0; i_1 < 10; i_1 += 1)
     {
-      t7[ip_1][i_1] = 0.0;
+      t6[ip_1][i_1] = 0.0;
+      t7[ip_1][i_1] = t6[ip_1][i_1];

     }

@@ -128,15 +123,10 @@
       double  t35  = 0.0;
       double  t34  = 0.0;

-      for (int  i_0  = 0; i_0 < 10; i_0 += 1)
-      {
-        t34 += t2[ip_0][i_0] * t6[ip_1][i_0];
-        
-      }
-      
       for (int  i_1  = 0; i_1 < 10; i_1 += 1)
       {
-        t35 += t2[ip_0][i_1] * t7[ip_1][i_1];
+        t34 += t2[ip_0][i_1] * t7[ip_1][i_1];
+        t35 += t34;

       }
       double  t36  = ((-1 * t34) + t35);

It seems that t6 and t7 are incorrectly recognised to be the same (they are not), thus t2[ip_0][i_0] * t6[ip_1][i_0] seems to be the same as t2[ip_0][i_1] * t7[ip_1][i_1], and even then the transformation in the lower hunk is just wrong.

[FabioLuporini]

t6 and t7 are "the same" in the upper hunk, meaning that they are both zero valued, so that substitution would be OK...

... However, there are two bugs here, and both are due to the replacement routine executed after loop fusion: 1) it doesn't distinguish across entirely different loop nests (this is horrible); 2) doesn't distinguish between AugmentedAssignment and Assign

Fortunately, both can trivially be fixed .

[miklos1]

To replace t7[ip_1][i_1] = 0.0; with t7[ip_1][i_1] = t6[ip_1][i_1]; is correct, because t6[ip_1][i_1] is still zero at that point. But later they are getting different values accumulated in them (which does not show up in the diff, but you can check the full unoptimised file on the link above), so assuming they are the same is incorrect for the sake of further analysis.

[FabioLuporini]

I think this is pretty much what I said above