8231055: C2: arraycopy with same non escaping src and dest but different positions causes wrong execution

Reviewed-by: thartmann, vlivanov

Author: rwestrel

src/hotspot/share/opto/macro.cpp

@@ -357,19 +357,38 @@ Node* PhaseMacroExpand::make_arraycopy_load(ArrayCopyNode* ac, intptr_t offset,
     if (ac->modifies(offset, offset, &_igvn, true)) {
       assert(ac->in(ArrayCopyNode::Dest) == alloc->result_cast(), "arraycopy destination should be allocation's result");
       uint shift = exact_log2(type2aelembytes(bt));
-      Node* diff = _igvn.transform(new SubINode(ac->in(ArrayCopyNode::SrcPos), ac->in(ArrayCopyNode::DestPos)));
+      Node* src_pos = ac->in(ArrayCopyNode::SrcPos);
+      Node* dest_pos = ac->in(ArrayCopyNode::DestPos);
+      const TypeInt* src_pos_t = _igvn.type(src_pos)->is_int();
+      const TypeInt* dest_pos_t = _igvn.type(dest_pos)->is_int();
+
+      Node* adr = NULL;
+      const TypePtr* adr_type = NULL;
+      if (src_pos_t->is_con() && dest_pos_t->is_con()) {
+        intptr_t off = ((src_pos_t->get_con() - dest_pos_t->get_con()) << shift) + offset;
+        Node* base = ac->in(ArrayCopyNode::Src);
+        adr = _igvn.transform(new AddPNode(base, base, MakeConX(off)));
+        adr_type = _igvn.type(base)->is_ptr()->add_offset(off);
+        if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) {
+          // Don't emit a new load from src if src == dst but try to get the value from memory instead
+          return value_from_mem(ac->in(TypeFunc::Memory), ctl, ft, ftype, adr_type->isa_oopptr(), alloc);
+        }
+      } else {
+        Node* diff = _igvn.transform(new SubINode(ac->in(ArrayCopyNode::SrcPos), ac->in(ArrayCopyNode::DestPos)));
 #ifdef _LP64
-      diff = _igvn.transform(new ConvI2LNode(diff));
+        diff = _igvn.transform(new ConvI2LNode(diff));
 #endif
-      diff = _igvn.transform(new LShiftXNode(diff, intcon(shift)));
-
-      Node* off = _igvn.transform(new AddXNode(MakeConX(offset), diff));
-      Node* base = ac->in(ArrayCopyNode::Src);
-      Node* adr = _igvn.transform(new AddPNode(base, base, off));
-      const TypePtr* adr_type = _igvn.type(base)->is_ptr()->add_offset(offset);
-      if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) {
-        // Don't emit a new load from src if src == dst but try to get the value from memory instead
-        return value_from_mem(ac->in(TypeFunc::Memory), ctl, ft, ftype, adr_type->isa_oopptr(), alloc);
+        diff = _igvn.transform(new LShiftXNode(diff, intcon(shift)));
+
+        Node* off = _igvn.transform(new AddXNode(MakeConX(offset), diff));
+        Node* base = ac->in(ArrayCopyNode::Src);
+        adr = _igvn.transform(new AddPNode(base, base, off));
+        adr_type = _igvn.type(base)->is_ptr()->add_offset(Type::OffsetBot);
+        if (ac->in(ArrayCopyNode::Src) == ac->in(ArrayCopyNode::Dest)) {
+          // Non constant offset in the array: we can't statically
+          // determine the value
+          return NULL;
+        }
       }
       res = LoadNode::make(_igvn, ctl, mem, adr, adr_type, type, bt, MemNode::unordered, LoadNode::UnknownControl);
     }

test/hotspot/jtreg/compiler/escapeAnalysis/TestSelfArrayCopy.java

@@ -23,7 +23,7 @@
 
 /*
  * @test
- * @bug 8229016
+ * @bug 8229016 8231055
  * @summary Test correct elimination of array allocation with arraycopy to itself.
  * @library /test/lib
  * @run main/othervm -Xbatch -XX:CompileCommand=compileonly,compiler.escapeAnalysis.TestSelfArrayCopy::test
@@ -39,7 +39,7 @@ public class TestSelfArrayCopy {
     private static final int rI1 = Utils.getRandomInstance().nextInt();
     private static final int rI2 = Utils.getRandomInstance().nextInt();
 
-    private static int test() {
+    private static int test1() {
         // Non-escaping allocation
         Integer[] array = {rI1, rI2};
         // Arraycopy with src == dst
@@ -51,14 +51,40 @@ private static int test() {
         return array[0] + array[1];
     }
 
+    private static int test2() {
+        // Non-escaping allocation
+        Integer[] array = {rI1, rI2};
+        // Arraycopy with src == dst
+        System.arraycopy(array, 0, array, 1, 1);
+        if (b) {
+            // Uncommon trap
+            System.out.println(array[0]);
+        }
+        return array[0] + array[1];
+    }
+
     public static void main(String[] args) {
-        int expected = rI1 + rI2;
+        int expected1 = rI1 + rI2;
+        int expected2 = rI1 + rI1;
         // Trigger compilation
         for (int i = 0; i < 20_000; ++i) {
-            int result = test();
-            if (result != expected) {
-                throw new RuntimeException("Incorrect result: " + result + " != " + expected);
+            int result = test1();
+            if (result != expected1) {
+                throw new RuntimeException("Incorrect result: " + result + " != " + expected1);
             }
+            result = test2();
+            if (result != expected2) {
+                throw new RuntimeException("Incorrect result: " + result + " != " + expected2);
+            }
+        }
+        b = true;
+        int result = test1();
+        if (result != expected1) {
+            throw new RuntimeException("Incorrect result: " + result + " != " + expected1);
+        }
+        result = test2();
+        if (result != expected2) {
+            throw new RuntimeException("Incorrect result: " + result + " != " + expected2);
         }
     }
 }