8382226: [lworld] C2: Fix _copyOf/_copyOfRange intrinsic for flat abstract value class arrays by chhagedorn · Pull Request #2569 · openjdk/valhalla

chhagedorn · 2026-06-19T12:34:36Z

The provided test cases fail when inlining the Array.copyOf/copyOfRange() intrinsics where the source array is flat and from an abstract value class.

The current code checks whether the source array or the destination array klass contain oops by assuming that a flat value class array is always concrete and thus an InlineKlass (i.e. can call inline_klass()). However, we could also have abstract value class arrays that are known to be flat (see test cases). This leads to a cast assertion failure because abstract value classes are represented by an InstanceKlass and not an InlineKlass.

To fix this, I added a simple bailout when detecting an abstract flat value class array. This is a conservative correctness fix and should be revisited again post-Valhalla-integration. We have JDK-8251971 in place for that which should also tackle other issues around the arraycopy intrinsics and also address performance problems.

Thanks,
Christian

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

JDK-8382226: [lworld] C2: Fix _copyOf/_copyOfRange intrinsic for flat abstract value class arrays (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/valhalla.git pull/2569/head:pull/2569
$ git checkout pull/2569

Update a local copy of the PR:
$ git checkout pull/2569
$ git pull https://git.openjdk.org/valhalla.git pull/2569/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2569

View PR using the GUI difftool:
$ git pr show -t 2569

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/valhalla/pull/2569.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-06-19T12:36:16Z

👋 Welcome back chagedorn! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-06-19T12:37:09Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

mlbridge · 2026-06-19T12:46:05Z

Webrevs

merykitty · 2026-06-19T13:04:43Z

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


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

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.

merykitty · 2026-06-19T14:41:49Z

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


This should also check for tklass->is_not_flat() instead.

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.

… read

merykitty · 2026-06-23T15:27:54Z

-                        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();
+    const bool can_dest_be_value_class_array = dest_klass_t != nullptr && dest_klass_t->can_be_inline_array();


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

merykitty · 2026-06-23T15:29:49Z

-                        // 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();


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.

fix

61240a2

openjdk Bot added the rfr Pull request is ready for review label Jun 19, 2026

merykitty reviewed Jun 19, 2026

View reviewed changes

chhagedorn added 2 commits June 19, 2026 15:11

Review Quan-Anh

bf89360

fix tklass

b1de11c

merykitty reviewed Jun 19, 2026

View reviewed changes

improve checks and further refactor exclude_flat to make it easier to…

e3b4f88

… read

merykitty reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

8382226: [lworld] C2: Fix _copyOf/_copyOfRange intrinsic for flat abstract value class arrays#2569
chhagedorn wants to merge 4 commits into
openjdk:lworldfrom
chhagedorn:JDK-8382226

chhagedorn commented Jun 19, 2026 •

edited by openjdk Bot

Loading

Uh oh!

bridgekeeper Bot commented Jun 19, 2026

Uh oh!

openjdk Bot commented Jun 19, 2026

Uh oh!

mlbridge Bot commented Jun 19, 2026 •

edited

Loading

Uh oh!

merykitty Jun 19, 2026

Uh oh!

chhagedorn Jun 19, 2026

Uh oh!

merykitty Jun 19, 2026

Uh oh!

chhagedorn Jun 20, 2026

Uh oh!

merykitty Jun 23, 2026

Uh oh!

merykitty Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

chhagedorn commented Jun 19, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper Bot commented Jun 19, 2026

Uh oh!

openjdk Bot commented Jun 19, 2026

Uh oh!

mlbridge Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

merykitty Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

chhagedorn Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

merykitty Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

chhagedorn Jun 20, 2026

Choose a reason for hiding this comment

Uh oh!

merykitty Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

merykitty Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

chhagedorn commented Jun 19, 2026 •

edited by openjdk Bot

Loading

mlbridge Bot commented Jun 19, 2026 •

edited

Loading