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 1 commit
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
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 is_src_abstract_flat_value_array = orig_t != nullptr && !orig_t->elem()->is_inlinetypeptr() && orig_t->is_flat();

@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.

Should we check for !orig_t->is_not_flat() instead?

@chhagedorn chhagedorn Jun 19, 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 a good idea to cover all the cases where it could be flat. Updated this and the check for tklass as well accordingly.

const bool can_dest_be_value_class_array = tklass->can_be_inline_array();
const bool is_dest_abstract_flat_value_class_array = can_dest_be_value_class_array && tklass->is_flat() &&
!tklass->is_aryklassptr()->elem()->is_instklassptr()->instance_klass()->is_inlinetype();
const bool is_abstract_flat_value_class_array_involved = is_src_abstract_flat_value_array || is_dest_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);
}
}