Skip to content

8387488: [lworld] C2: acmp expansion fails with assert(replace != nullptr) failed: must succeed #2602

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
TobiHartmann wants to merge 5 commits into
base: lworld
Choose a base branch
from
+56 −12
Open

8387488: [lworld] C2: acmp expansion fails with assert(replace != nullptr) failed: must succeed #2602

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
828c4c0
Prototype
TobiHartmann Jun 30, 2026
d0f66d9
Workaround for C1 issue
TobiHartmann Jun 30, 2026
6adb893
Found another missing TODO
TobiHartmann Jun 30, 2026
1016b74
Review comments
TobiHartmann Jun 30, 2026
65d1515
New attempt at comment
TobiHartmann 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
35 changes: 23 additions & 12 deletions src/hotspot/share/opto/inlinetypenode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,6 +865,8 @@ Node* InlineTypeNode::emit_substitutability_check(GraphKit* kit, Node* lhs, Node

Node* cur_lhs_field = cur_lhs->field_value(field_idx);
Node* cur_rhs_field = cur_rhs->field_value(field_idx);
InlineTypeNode* cur_lhs_inline = cur_lhs_field->isa_InlineType();
InlineTypeNode* cur_rhs_inline = cur_rhs_field->isa_InlineType();

Node* preprocess = emit_substitutability_check_pointer(kit, result, cur_lhs_field, cur_rhs_field);
if (kit->stopped()) {
Expand Down Expand Up @@ -951,21 +953,30 @@ Node* InlineTypeNode::emit_substitutability_check(GraphKit* kit, Node* lhs, Node
Node* vk_klass = igvn.makecon(vk_klass_type);

// Both must be vk
Node* not_vk = kit->top();
cur_lhs_field = kit->gen_checkcast(cur_lhs_field, vk_klass, &not_vk);
if (!not_vk->is_top()) {
region->add_req(not_vk);
result->add_req(igvn.intcon(0));
// InlineTypeNodes need no (new) field loads, so we can use the uncasted value below.
if (cur_lhs_inline != nullptr && cur_lhs_inline->inline_klass() == vk) {

@merykitty merykitty Jun 30, 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.

You can add a comment that it is not an issue if cur_lhs_inline is nullable and cur_lhs_field is null-checked, it is null-checked so that we can load the fields, and an InlineTypeNode does that already.

@merykitty merykitty Jun 30, 2026 •
edited
Loading

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.

It's not quite what I want to say. The null check is still present, the thing is that we ignored the null-checked value (the CastPP) and just use the raw value. This is because normally, we need a non-null CastPP to load from the object. That is unnecessary for an InlineTypeNode because we don't perform the loads.

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

Okay, here's a new attempt 🙂

cur_lhs_field = cur_lhs_inline;
} else {
Node* not_vk = kit->top();
cur_lhs_field = kit->gen_checkcast(cur_lhs_field, vk_klass, &not_vk);
if (!not_vk->is_top()) {
region->add_req(not_vk);
result->add_req(igvn.intcon(0));
}
cur_lhs_field = InlineTypeNode::make_from_oop(kit, cur_lhs_field, vk);
}
cur_lhs_field = InlineTypeNode::make_from_oop(kit, cur_lhs_field, vk);

not_vk = kit->top();
cur_rhs_field = kit->gen_checkcast(cur_rhs_field, vk_klass, &not_vk);
if (!not_vk->is_top()) {
region->add_req(not_vk);
result->add_req(igvn.intcon(0));
if (cur_rhs_inline != nullptr && cur_rhs_inline->inline_klass() == vk) {
cur_rhs_field = cur_rhs_inline;
} else {
Node* not_vk = kit->top();
cur_rhs_field = kit->gen_checkcast(cur_rhs_field, vk_klass, &not_vk);
if (!not_vk->is_top()) {
region->add_req(not_vk);
result->add_req(igvn.intcon(0));
}
cur_rhs_field = InlineTypeNode::make_from_oop(kit, cur_rhs_field, vk);
}
cur_rhs_field = InlineTypeNode::make_from_oop(kit, cur_rhs_field, vk);

if (!kit->stopped()) {
// Push the expanded InlineTypeNodes for processing later
Expand Down
32 changes: 32 additions & 0 deletions test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestLWorld.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5479,4 +5479,36 @@ public static void testBadCastToInlineKlass_verifier() {
// expected
}
}

// TODO 8376254: C1 bails out if the type of the nullable flat field is uninitialized
static ObjectHolderHolder objectHolderHolder = new ObjectHolderHolder();

static value class ObjectHolder {
Object obj = new Object();
}

static value class ObjectHolderHolder {
ObjectHolder objectHolder = new ObjectHolder();
}

// Test that acmp expansion works with field of exact Object type
@Test
@IR(failOn = {STATIC_CALL_OF_METHOD, "isSubstitutable.*"},
counts = {IRNode.ALLOC, "= 2"}) // The two Object allocations
static boolean testAcmp() {
Object lhs = null;
Object rhs = null;
for (int i = 0; i < 10; i++) {
if ((i & 1) == 0) {
lhs = new ObjectHolderHolder();
rhs = new ObjectHolderHolder();
}
}
return lhs == rhs;
}

@Run(test = "testAcmp")
public void runTestAcmp() {
Asserts.assertFalse(testAcmp());
}
}
1 change: 1 addition & 0 deletions test/hotspot/jtreg/compiler/valhalla/inlinetypes/TestOptimizePtrCompare.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ static MyValue2 notInlined5(MyValue2 v) {
return v;
}

// TODO 8376254: C1 bails out if the type of the nullable flat field is uninitialized

@TobiHartmann TobiHartmann Jun 30, 2026 •
edited
Loading

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.

Just found this missing TODO when browsing code.

@Test(allowNotCompilable = true)
@IR(applyIfAnd = {"InlineTypePassFieldsAsArgs", "true", "InlineTypeReturnedAsFields", "true"}, failOn = {IRNode.CMP_P_OR_N})
@IR(applyIfAnd = {"InlineTypePassFieldsAsArgs", "false", "InlineTypeReturnedAsFields", "true"}, counts = {IRNode.CMP_P_OR_N, "4", IRNode.CMP_I, "2"})
Expand Down