CSE, loops, and array accesses

[Edward Nevill]

On Thu, 2015-10-01 at 11:27 +0100, Andrew Haley wrote:
> I'm looking at a failure to CSE the array base offset in this loop:
> 
>         for (int i = 0; i < array.length; i++) {
>             res += array[i];
>         }
> 
>  ;; B2: #       B2 B3 <- B1 B2  Loop: B2-B2 inner  Freq: 100.958
> 
>   0x000003ff9d1a1a30: add       xscratch1, x10, #0x10
>   0x000003ff9d1a1a34: ldr       w13, [xscratch1,w11,sxtw #2]
>   0x000003ff9d1a1a38: add       w11, w11, #0x1  ;*iinc
>                                                 ; - tests.ScalarReduce::test1 at 20 (line 22)
> 
>   0x000003ff9d1a1a3c: add       w0, w0, w13     ;*iadd
>                                                 ; - tests.ScalarReduce::test1 at 18 (line 23)
> 
>   0x000003ff9d1a1a40: cmp       w11, #0x64
>   0x000003ff9d1a1a44: b.lt      0x000003ff9d1a1a30  ;*if_icmpge
>                                                 ; - tests.ScalarReduce::test1 at 9 (line 22)

The reason it does not do the loop invariant lifting is because of the following transformation in AddPNode::Ideal in addnode.cpp

        // Else move the constant to the right.  ((A+con)+B) into ((A+B)+con)
        address = phase->transform(new (phase->C) AddPNode(in(Base),addp->in(Address),in(Offset)));
        offset  = addp->in(Offset);

So, once it has transformed ((A+con)+B) into ((A+B)+con) all bets are off, we can never CSE/Loop invariant lift.

Try the following patch, it should generate the code you want.

All the best,
Ed.

--- CUT HERE ---
diff -r fa430fa4f577 src/share/vm/opto/addnode.cpp
--- a/src/share/vm/opto/addnode.cpp	Wed Sep 23 12:39:30 2015 -0400
+++ b/src/share/vm/opto/addnode.cpp	Thu Oct 01 12:59:19 2015 +0000
@@ -592,20 +592,16 @@
         // The Add of the flattened expression
         address = addp->in(Address);
         offset  = phase->MakeConX(t2->get_con() + t12->get_con());
-      } else {
-        // Else move the constant to the right.  ((A+con)+B) into ((A+B)+con)
-        address = phase->transform(new AddPNode(in(Base),addp->in(Address),in(Offset)));
-        offset  = addp->in(Offset);
+        PhaseIterGVN *igvn = phase->is_IterGVN();
+        if( igvn ) {
+          set_req_X(Address,address,igvn);
+          set_req_X(Offset,offset,igvn);
+        } else {
+          set_req(Address,address);
+          set_req(Offset,offset);
+        }
+        return this;
       }
-      PhaseIterGVN *igvn = phase->is_IterGVN();
-      if( igvn ) {
-        set_req_X(Address,address,igvn);
-        set_req_X(Offset,offset,igvn);
-      } else {
-        set_req(Address,address);
-        set_req(Offset,offset);
-      }
-      return this;
     }
   }
 
--- CUT HERE ---