8382226: [lworld] C2: Fix _copyOf/_copyOfRange intrinsic for flat abstract value class arrays

src/hotspot/share/opto/library_call.cpp

@@ -5177,32 +5177,33 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
 
     Node* orig_length = load_array_length(original);
 
-    Node* klass_node = load_klass_from_mirror(array_type_mirror, false, nullptr, 0);
+    RegionNode* bailout = new RegionNode(2);
+    record_for_igvn(bailout);
+
+    Node* klass_node = load_klass_from_mirror(array_type_mirror, false, bailout, 1);
+    if (stopped()) {
+      // Arrays.copyOf() uses a generic Class parameter which is erased to the raw type Class. This also allows
+      // passing in primitive class mirrors like int.class which do not have corresponding Klass* pointers.
+      // In these cases, klass_node will be top. Emit a trap to throw in the interpreter in this case.
+      bail_out_from_array_copyOf(bailout);
+      return true;
+    }
+
     klass_node = null_check(klass_node);
 
-    RegionNode* bailout = new RegionNode(1);
-    record_for_igvn(bailout);
+    const TypeAryPtr* src_t = _gvn.type(original)->is_aryptr();
+    const TypeKlassPtr* dest_klass_t = _gvn.type(klass_node)->is_klassptr()->is_klassptr();
 
-    // Despite the generic type of Arrays.copyOf, the mirror might be int, int[], etc.
-    // Bail out if that is so.
-    // Inline type array may have object field that would require a
-    // write barrier. Conservatively, go to slow path.
-    // TODO 8251971: Optimize for the case when flat src/dst are later found
-    // to not contain oops (i.e., move this check to the macro expansion phase).
-    // TODO 8382226: Revisit for flat abstract value class arrays
-    BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
-    const TypeAryPtr* orig_t = _gvn.type(original)->isa_aryptr();
-    const TypeKlassPtr* tklass = _gvn.type(klass_node)->is_klassptr();
-    bool exclude_flat = UseArrayFlattening && bs->array_copy_requires_gc_barriers(true, T_OBJECT, false, false, BarrierSetC2::Parsing) &&
-                        // Can src array be flat and contain oops?
-                        (orig_t == nullptr || (!orig_t->is_not_flat() && (!orig_t->is_flat() || orig_t->elem()->inline_klass()->contains_oops()))) &&
-                        // Can dest array be flat and contain oops?
-                        tklass->can_be_inline_array() && (!tklass->is_flat() || tklass->is_aryklassptr()->elem()->is_instklassptr()->instance_klass()->as_inline_klass()->contains_oops());
-    Node* not_objArray = exclude_flat ? generate_non_refArray_guard(klass_node, bailout) : generate_typeArray_guard(klass_node, bailout);
+    Node* success_proj;
+    if (should_bail_out_on_non_ref_arrays(src_t, dest_klass_t)) {
+      success_proj = generate_non_refArray_guard(klass_node, bailout);
+    } else {
+      success_proj = generate_typeArray_guard(klass_node, bailout);
+    }
 
     Node* refined_klass_node = load_default_refined_array_klass(klass_node, /* type_array_guard= */ false);
 
-    if (not_objArray != nullptr) {
+    if (success_proj != nullptr) {
       // Improve the klass node's type from the new optimistic assumption:
       ciKlass* ak = ciArrayKlass::make(env()->Object_klass());
       bool not_flat = !UseArrayFlattening;
@@ -5238,10 +5239,7 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
     generate_negative_guard(orig_tail, bailout, &orig_tail);
 
     if (bailout->req() > 1) {
-      PreserveJVMState pjvms(this);
-      set_control(_gvn.transform(bailout));
-      uncommon_trap(Deoptimization::Reason_intrinsic,
-                    Deoptimization::Action_maybe_recompile);
+      bail_out_from_array_copyOf(bailout);
     }
 
     if (!stopped()) {
@@ -5319,6 +5317,60 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
   return true;
 }
 
+void LibraryCallKit::bail_out_from_array_copyOf(RegionNode* bailout_region) {
+  PreserveJVMState pjvms(this);
+  set_control(_gvn.transform(bailout_region));
+  uncommon_trap(Deoptimization::Reason_intrinsic,
+                Deoptimization::Action_maybe_recompile);
+}
+
+bool LibraryCallKit::should_bail_out_on_non_ref_arrays(const TypeAryPtr* src_type, const TypeKlassPtr* dest_klass_type) {
+  const TypeAryKlassPtr* dest_ary_klass_type = dest_klass_type->isa_aryklassptr();
+  if (dest_ary_klass_type == nullptr) {
+    // Dest klass is not known to be an array class. There are multiple cases:
+    // - Primitive class mirror: We already bailed out before.
+    // - Instance class mirror: We should bail out.
+    // - java.lang.Object (possible due to type erasure): Could be anything including primitive or instance class mirror
+    //   or also flat arrays. Bail out.
+    return true;
+  }
+
+  if (UseArrayFlattening) {
+    // The remaining checks revolve around array flatness. Without array flatness, we don't need the stronger non-ref
+    // runtime check excluding flat arrays.
+    return false;
+  }
+
+  // We now know that src and dest are proper array pointers.
+  const bool src_maybe_flat = !src_type->is_not_flat();
+  const bool dest_maybe_flat = !dest_ary_klass_type->is_not_flat();
+
+  // We could have abstract flat value class arrays whose layout we don't know. Bail out.
+  const bool can_src_be_abstract_flat_value_class_array = src_maybe_flat && !src_type->elem()->is_inlinetypeptr();
+  const bool can_dest_be_abstract_flat_value_class_array = dest_maybe_flat &&
+                                                           !dest_ary_klass_type->elem()->is_instklassptr()->instance_klass()->is_inlinetype();
+  if (can_src_be_abstract_flat_value_class_array || can_dest_be_abstract_flat_value_class_array) {
+    return true;
+  }
+
+  // Value class array may have object field that would require a write barrier. Conservatively bail out.
+  // TODO 8251971: Optimize for the case when flat src/dst are later found to not contain
+  //               oops (i.e., move this check to the macro expansion phase).
+  BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
+  if (bs->array_copy_requires_gc_barriers(true, T_OBJECT, false, false, BarrierSetC2::Parsing)) {
+    // No barriers required.
+    return false;
+  }
+
+  const bool can_src_be_flat_with_oops = src_maybe_flat && src_type->elem()->inline_klass()->contains_oops();
+  const bool can_dest_be_flat_with_oops = dest_maybe_flat && dest_ary_klass_type->elem()->is_instklassptr()->instance_klass()->as_inline_klass()->contains_oops();
+  if (can_src_be_flat_with_oops || can_dest_be_flat_with_oops) {
+    return true;
+  }
+
+  // Can handle remaining flat arrays.
+  return false;
+}
 
 //----------------------generate_virtual_guard---------------------------
 // Helper for hashCode and clone.  Peeks inside the vtable to avoid a call.

src/hotspot/share/opto/library_call.hpp

@@ -304,6 +304,8 @@ class LibraryCallKit : public GraphKit {
   bool inline_native_subtype_check();
   bool inline_native_getLength();
   bool inline_array_copyOf(bool is_copyOfRange);
+  void bail_out_from_array_copyOf(RegionNode* bailout_region);
+  static bool should_bail_out_on_non_ref_arrays(const TypeAryPtr* src_type, const TypeKlassPtr* dest_klass_type);
   bool inline_array_equals(StrIntrinsicNode::ArgEnc ae);
   bool inline_preconditions_checkIndex(BasicType bt);
   void copy_to_clone(Node* obj, Node* alloc_obj, Node* obj_size, bool is_array);

test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestArrays.java

@@ -3901,10 +3901,50 @@ static value class BadCastV2 extends BadCastA {
         }
     }
 
+    @LooselyConsistentValue
+    static value class BadCastV3 {
+        byte a;
+        byte b;
+        byte c;
+        byte d;
+
+        BadCastV3(int i) {
+            a = (byte)i;
+            b = (byte)(i + 1);
+            c = (byte)(i + 2);
+            d = (byte)(i + 3);
+        }
+
+        BadCastV3() {
+            this(1);
+        }
+    }
+
+    @LooselyConsistentValue
+    static value class BadCastV4 {
+        byte a;
+        byte b;
+        byte c;
+        byte d;
+
+        BadCastV4(int i) {
+            a = (byte)i;
+            b = (byte)(i + 1);
+            c = (byte)(i + 2);
+            d = (byte)(i + 3);
+        }
+
+        BadCastV4() {
+            this(1);
+        }
+    }
+
     static BadCastV1 badCastv1 = new BadCastV1(11);
     static BadCastV1 badCastv11 =  new BadCastV1(21);
     static BadCastV2 badCastv2 =  new BadCastV2(31);
     static BadCastV2 badCastv22 =  new BadCastV2(41);
+    static BadCastV3 badCastv3 = new BadCastV3(11);
+    static BadCastV4 badCastv4 = new BadCastV4(11);
     static int badCastLen = Math.abs(rI) % 10;
 
     @Test
@@ -3959,10 +3999,101 @@ static BadCastA[] testBadCastAbstractArray4(boolean flag) {
         return aArr2;
     }
 
+    // Needs -XX:-ReduceInitialCardMarks to trigger.
+    @Test
+    static BadCastA[] testBadCastAbstractArray5(boolean flag) {
+        BadCastA[] aArr;
+        if (flag) {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV1.class, badCastLen, badCastv1);
+        } else {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV2.class, badCastLen, badCastv2);
+        }
+
+        return Arrays.copyOf(aArr, badCastLen, BadCastA[].class);
+    }
+
+    @Test
+    static BadCastA[] testBadCastAbstractArray5a(boolean flag) {
+        BadCastA[] aArr;
+        if (flag) {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV1.class, badCastLen, badCastv1);
+        } else {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV2.class, badCastLen, badCastv2);
+        }
+
+        return Arrays.copyOfRange(aArr, 0, badCastLen, BadCastA[].class);
+    }
+
+    @Test
+    static BadCastA[] testBadCastAbstractArray6(boolean flag) {
+        BadCastA[] aArr;
+        Class<? extends BadCastA[]> c;
+        if (flag) {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV1.class, badCastLen, badCastv1);
+            c = BadCastV1[].class;
+        } else {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV2.class, badCastLen, badCastv2);
+            c = BadCastV2[].class;
+        }
+
+        return Arrays.copyOf(aArr, badCastLen, c);
+    }
+
+    @Test
+    static BadCastA[] testBadCastAbstractArray6a(boolean flag) {
+        BadCastA[] aArr;
+        Class<? extends BadCastA[]> c;
+        if (flag) {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV1.class, badCastLen, badCastv1);
+            c = BadCastV1[].class;
+        } else {
+            aArr = (BadCastA[]) ValueClass.newNullRestrictedNonAtomicArray(BadCastV2.class, badCastLen, badCastv2);
+            c = BadCastV2[].class;
+        }
+
+        return Arrays.copyOfRange(aArr, 0, badCastLen, c);
+    }
+
+    @Test
+    static Object[] testBadCastAbstractArray7(boolean flag) {
+        Object[] oArr;
+        Class<? extends Object[]> c;
+        if (flag) {
+            oArr = ValueClass.newNullRestrictedNonAtomicArray(BadCastV3.class, badCastLen, badCastv3);
+            c = BadCastV3[].class;
+        } else {
+            oArr = ValueClass.newNullRestrictedNonAtomicArray(BadCastV4.class, badCastLen, badCastv4);
+            c = BadCastV4[].class;
+        }
+
+        return Arrays.copyOf(oArr, badCastLen, c);
+    }
+
+    @Test
+    static Object[] testBadCastAbstractArray7a(boolean flag) {
+        Object[] oArr;
+        Class<? extends Object[]> c;
+        if (flag) {
+            oArr = ValueClass.newNullRestrictedNonAtomicArray(BadCastV3.class, badCastLen, badCastv3);
+            c = BadCastV3[].class;
+        } else {
+            oArr = ValueClass.newNullRestrictedNonAtomicArray(BadCastV4.class, badCastLen, badCastv4);
+            c = BadCastV4[].class;
+        }
+
+        return Arrays.copyOfRange(oArr, 0, badCastLen, c);
+    }
+
     @Run(test = {"testBadCastAbstractArray1",
                  "testBadCastAbstractArray2",
                  "testBadCastAbstractArray3",
-                 "testBadCastAbstractArray4"})
+                 "testBadCastAbstractArray4",
+                 "testBadCastAbstractArray5",
+                 "testBadCastAbstractArray5a",
+                 "testBadCastAbstractArray6",
+                 "testBadCastAbstractArray6a",
+                 "testBadCastAbstractArray7",
+                 "testBadCastAbstractArray7a"})
     @Warmup(0)
     static void testBadCastAbstractArray_verifier() {
         boolean flag = true;
@@ -3996,6 +4127,37 @@ static void testBadCastAbstractArray_verifier() {
             for (int j = 0; j < badCastLen; ++j) {
                 Asserts.assertEQ(src[j], dst[j]);
             }
+
+            dst = testBadCastAbstractArray5(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(src[j], dst[j]);
+            }
+
+            dst = testBadCastAbstractArray5a(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(src[j], dst[j]);
+            }
+
+            dst = testBadCastAbstractArray6(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(src[j], dst[j]);
+            }
+
+            dst = testBadCastAbstractArray6a(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(src[j], dst[j]);
+            }
+
+            Object[] srcObj = flag ? resetBadCastV3() : resetBadCastV4();
+            Object[] dstObj = testBadCastAbstractArray7(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(srcObj[j], dstObj[j]);
+            }
+
+            dstObj = testBadCastAbstractArray7a(flag);
+            for (int j = 0; j < badCastLen; ++j) {
+                Asserts.assertEQ(srcObj[j], dstObj[j]);
+            }
             flag = !flag;
         }
     }
@@ -4015,4 +4177,12 @@ static BadCastA[] resetBadCastV2() {
     static BadCastA[] resetBadCastV22() {
         return (BadCastA[])ValueClass.newNullRestrictedNonAtomicArray(BadCastV2.class, badCastLen, badCastv22);
     }
+
+    static Object[] resetBadCastV3() {
+        return ValueClass.newNullRestrictedNonAtomicArray(BadCastV3.class, badCastLen, badCastv3);
+    }
+
+    static Object[] resetBadCastV4() {
+        return ValueClass.newNullRestrictedNonAtomicArray(BadCastV4.class, badCastLen, badCastv4);
+    }
 }