changeset 57457:be9033a248f7

8233164: C2 fails with assert(phase->C->get_alias_index(t) == phase->C->get_alias_index(t_adr)) failed: correct memory chain Summary: Use _src_type/_dest_type as address types for the loads and stores. Reviewed-by: vlivanov, roland
author thartmann
date Mon, 23 Dec 2019 09:44:35 +0100
parents 563fa900fa17
children 97744abc4fde
files src/hotspot/share/opto/arraycopynode.cpp src/hotspot/share/opto/arraycopynode.hpp src/hotspot/share/opto/memnode.cpp test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyMemoryChain.java
diffstat 4 files changed, 95 insertions(+), 10 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/opto/arraycopynode.cpp	Sat Dec 21 13:03:02 2019 +0100
+++ b/src/hotspot/share/opto/arraycopynode.cpp	Mon Dec 23 09:44:35 2019 +0100
@@ -348,13 +348,12 @@
   return true;
 }
 
-const TypePtr* ArrayCopyNode::get_address_type(PhaseGVN *phase, Node* n) {
-  const Type* at = phase->type(n);
-  assert(at != Type::TOP, "unexpected type");
-  const TypePtr* atp = at->isa_ptr();
+const TypePtr* ArrayCopyNode::get_address_type(PhaseGVN* phase, const TypePtr* atp, Node* n) {
+  if (atp == TypeOopPtr::BOTTOM) {
+    atp = phase->type(n)->isa_ptr();
+  }
   // adjust atp to be the correct array element address type
-  atp = atp->add_offset(Type::OffsetBot);
-  return atp;
+  return atp->add_offset(Type::OffsetBot);
 }
 
 void ArrayCopyNode::array_copy_test_overlap(PhaseGVN *phase, bool can_reshape, bool disjoint_bases, int count, Node*& forward_ctl, Node*& backward_ctl) {
@@ -574,8 +573,8 @@
 
   Node* src = in(ArrayCopyNode::Src);
   Node* dest = in(ArrayCopyNode::Dest);
-  const TypePtr* atp_src = get_address_type(phase, src);
-  const TypePtr* atp_dest = get_address_type(phase, dest);
+  const TypePtr* atp_src = get_address_type(phase, _src_type, src);
+  const TypePtr* atp_dest = get_address_type(phase, _dest_type, dest);
 
   Node *in_mem = in(TypeFunc::Memory);
   if (!in_mem->is_MergeMem()) {
--- a/src/hotspot/share/opto/arraycopynode.hpp	Sat Dec 21 13:03:02 2019 +0100
+++ b/src/hotspot/share/opto/arraycopynode.hpp	Mon Dec 23 09:44:35 2019 +0100
@@ -90,7 +90,7 @@
 
   intptr_t get_length_if_constant(PhaseGVN *phase) const;
   int get_count(PhaseGVN *phase) const;
-  static const TypePtr* get_address_type(PhaseGVN *phase, Node* n);
+  static const TypePtr* get_address_type(PhaseGVN* phase, const TypePtr* atp, Node* n);
 
   Node* try_clone_instance(PhaseGVN *phase, bool can_reshape, int count);
   bool prepare_array_copy(PhaseGVN *phase, bool can_reshape,
--- a/src/hotspot/share/opto/memnode.cpp	Sat Dec 21 13:03:02 2019 +0100
+++ b/src/hotspot/share/opto/memnode.cpp	Mon Dec 23 09:44:35 2019 +0100
@@ -207,7 +207,7 @@
   Node* result = optimize_simple_memory_chain(mchain, t_oop, load, phase);
   bool is_instance = t_oop->is_known_instance_field();
   PhaseIterGVN *igvn = phase->is_IterGVN();
-  if (is_instance && igvn != NULL  && result->is_Phi()) {
+  if (is_instance && igvn != NULL && result->is_Phi()) {
     PhiNode *mphi = result->as_Phi();
     assert(mphi->bottom_type() == Type::MEMORY, "memory phi required");
     const TypePtr *t = mphi->adr_type();
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/arraycopy/TestArrayCopyMemoryChain.java	Mon Dec 23 09:44:35 2019 +0100
@@ -0,0 +1,86 @@
+/*
+ * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8233164
+ * @summary Test correct wiring of load/store memory for arraycopy ideal transformation.
+ * @run main/othervm -XX:CompileCommand=dontinline,compiler.arraycopy.TestArrayCopyMemoryChain::test* -Xbatch
+ *                   compiler.arraycopy.TestArrayCopyMemoryChain
+ */
+
+package compiler.arraycopy;
+
+public class TestArrayCopyMemoryChain {
+
+    private String mySweetEscape1 = null;
+
+    private String getString(int i) {
+        return "A" + i + "B";
+    }
+
+    // Original test depending on Indify String Concat
+    public void test1(int i) {
+        mySweetEscape1 = getString(i) + "CD";
+    }
+
+    private byte[] mySweetEscape2;
+
+    class Wrapper {
+        public final byte[] array;
+        public Wrapper(byte[] array) {
+            this.array = array;
+        }
+    }
+
+    // Simplified test independent of Strings
+    public void test2(int idx, int size) {
+        // Create destination array with unknown size and let it escape.
+        byte[] dst = new byte[size];
+        mySweetEscape2 = dst;
+        // Create constant src1 array.
+        byte[] src1 = {43, 44};
+        // Wrap src2 into an Object such that it's only available after
+        // Escape Analys determined that the Object is non-escaping.
+        byte[] array = {42};
+        Wrapper wrapper = new Wrapper(array);
+        byte[] src2 = wrapper.array;
+        // Copy src1 and scr2 into destination array.
+        System.arraycopy(src1, 0, dst, idx, src1.length);
+        System.arraycopy(src2, 0, dst, 0, src2.length);
+    }
+
+    public static void main(String[] args) {
+        TestArrayCopyMemoryChain t = new TestArrayCopyMemoryChain();
+        for (int i = 0; i < 100_000; ++i) {
+            t.test1(0);
+            if (!t.mySweetEscape1.equals("A0BCD")) {
+                throw new RuntimeException("Test1 failed");
+            }
+            t.test2(1, 3);
+            if (t.mySweetEscape2[0] != 42 || t.mySweetEscape2[1] != 43 || t.mySweetEscape2[2] != 44) {
+                throw new RuntimeException("Test2 failed");
+            }
+        }
+    }
+}