Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chhagedorn wants to merge 4 commits into
base: lworld
Choose a base branch
from
+193 −12
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
61240a2
fix
chhagedorn Jun 19, 2026
bf89360
Review Quan-Anh
chhagedorn Jun 19, 2026
b1de11c
fix tklass
chhagedorn Jun 19, 2026
e3b4f88
improve checks and further refactor exclude_flat to make it easier to…
chhagedorn Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 22 additions & 11 deletions src/hotspot/share/opto/library_call.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5193,19 +5193,30 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {

// 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());
const TypeAryKlassPtr* dest_klass_t = _gvn.type(klass_node)->is_klassptr()->isa_aryklassptr();
const bool can_src_be_abstract_flat_value_class_array = orig_t != nullptr && !orig_t->elem()->is_inlinetypeptr() && !orig_t->is_not_flat();

@merykitty merykitty Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is a little misleading, I think you want to catch the case orig_t == nullptr below, but if the static type is j.l.O, the runtime type can still be an abstract flat array.

const bool can_dest_be_value_class_array = dest_klass_t != nullptr && dest_klass_t->can_be_inline_array();

@merykitty merykitty Jun 23, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if dest is not an aryklassptr? I think in that case this should also be true, right?

const bool can_dest_be_abstract_flat_value_class_array = can_dest_be_value_class_array && !dest_klass_t->is_not_flat() &&
!dest_klass_t->elem()->is_instklassptr()->instance_klass()->is_inlinetype();
const bool is_abstract_flat_value_class_array_involved = can_src_be_abstract_flat_value_class_array || can_dest_be_abstract_flat_value_class_array;
const bool can_src_be_flat_with_oops = orig_t == nullptr ||
(!can_src_be_abstract_flat_value_class_array &&
!orig_t->is_not_flat() && (!orig_t->is_flat() || orig_t->elem()->inline_klass()->contains_oops()));
const bool can_dest_be_flat_with_oops = can_dest_be_value_class_array && !can_dest_be_abstract_flat_value_class_array &&
!dest_klass_t->is_not_flat() &&
(!dest_klass_t->is_flat() || dest_klass_t->elem()->is_instklassptr()->instance_klass()->as_inline_klass()->contains_oops());

const bool exclude_flat = UseArrayFlattening &&
// We do not know the exact layout of an abstract flat value class array. Bail out.
(is_abstract_flat_value_class_array_involved ||
// Value class 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).
(bs->array_copy_requires_gc_barriers(true, T_OBJECT, false, false, BarrierSetC2::Parsing) &&
can_src_be_flat_with_oops && can_dest_be_flat_with_oops));
Node* not_objArray = exclude_flat ? generate_non_refArray_guard(klass_node, bailout) : generate_typeArray_guard(klass_node, bailout);

Node* refined_klass_node = load_default_refined_array_klass(klass_node, /* type_array_guard= */ false);
Expand Down
172 changes: 171 additions & 1 deletion test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestArrays.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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);
}
}