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 11 commits into
base: lworld
Choose a base branch
from
+636 −25
Open

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 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
0819467
Update
chhagedorn Jun 26, 2026
2868844
Fix endless deopts with -XX:-UseArrayFlattening
chhagedorn Jun 29, 2026
da29a68
Merge branch 'lworld' into JDK-8382226
chhagedorn Jun 29, 2026
02fdea6
Update
chhagedorn Jun 30, 2026
30400c2
Merge branch 'lworld' into JDK-8382226
chhagedorn Jun 30, 2026
53493bc
Improve comments
chhagedorn Jun 30, 2026
20cd921
Add test for void.class as well
chhagedorn Jul 1, 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
13 changes: 10 additions & 3 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 @@ -5197,15 +5197,22 @@ bool LibraryCallKit::inline_array_copyOf(bool is_copyOfRange) {
// 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) &&
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.

@chhagedorn chhagedorn Jul 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That's true. Though, I'm wondering if orig_t could even be a non-array pointer at all. The Arrays.copyOf() takes a U[], so after type erasure, we should know it's an Object[] and thus have an array
pointer. It looks like the original code was just defensive by checking orig_t as well. I removed it and testing passed.

const bool can_dest_be_value_class_array = tklass->can_be_inline_array();
const bool can_dest_be_abstract_flat_value_class_array = can_dest_be_value_class_array && !tklass->is_not_flat() && tklass->isa_aryklassptr() != nullptr &&
!tklass->is_aryklassptr()->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 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 ||
(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());
can_dest_be_value_class_array && (!tklass->is_flat() || tklass->is_aryklassptr()->elem()->is_instklassptr()->instance_klass()->as_inline_klass()->contains_oops())));

@merykitty merykitty Jun 19, 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 should also check for tklass->is_not_flat() instead.

@chhagedorn chhagedorn Jun 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I agree. I also took another look at the code again and refactored it a bit to make exclude_flat easier to understand. Let me know, what you think.

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);
}
}