Skip to content

8357474: [lworld] Consolidate load/store flat inline types #1470

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

Closed
wants to merge 12 commits into from
Closed

8357474: [lworld] Consolidate load/store flat inline types #1470

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cd74d47
Consolidate load/store of flat values
merykitty May 21, 2025
09d4dda
add tests
merykitty May 21, 2025
8173ef2
convert_from_payload
merykitty May 21, 2025
99d2269
array
merykitty May 22, 2025
1029080
fix array field calculations
merykitty May 22, 2025
b151571
fix array of nullable empty type
merykitty May 22, 2025
77f145d
generalize weird layout
merykitty May 23, 2025
5ac917f
typo
merykitty May 23, 2025
f7e3bd3
weird layout
merykitty May 23, 2025
a364b7a
cast_to_flat_array
merykitty May 26, 2025
e49bbc8
Merge branch 'lworld' into loadstoreflat
merykitty May 26, 2025
0a945fe
remove ptr_type parameters
merykitty May 30, 2025
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
Prev Previous commit
Next Next commit
cast_to_flat_array
  • Loading branch information
@merykitty
merykitty committed May 26, 2025
commit a364b7a1fdb43e9b7bf7262d7b85b608754ff8f9
23 changes: 18 additions & 5 deletions src/hotspot/share/opto/graphKit.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1871,14 +1871,27 @@ Node* GraphKit::array_element_address(Node* ary, Node* idx, BasicType elembt,
return basic_plus_adr(ary, base, scale);
}

Node* GraphKit::flat_array_element_address(Node*& array, Node* idx, ciInlineKlass* vk, bool is_null_free,
bool is_not_null_free, bool is_atomic) {
Node* GraphKit::cast_to_flat_array(Node* array, ciInlineKlass* vk, bool is_null_free, bool is_not_null_free, bool is_atomic) {
assert(vk->maybe_flat_in_array(), "element of type %s cannot be flat in array", vk->name()->as_utf8());
if (!vk->has_nullable_atomic_layout()) {
// Element does not have a nullable flat layout, cannot be nullable
is_null_free = true;
}
if (!vk->has_atomic_layout() && !vk->has_non_atomic_layout()) {
// Element does not have a null-free flat layout, cannot be null-free
is_not_null_free = true;
}
if (is_null_free) {
// TODO 8350865 Impossible type
is_not_null_free = false;
}

bool is_exact = is_null_free || is_not_null_free;
ciArrayKlass* array_klass = ciArrayKlass::make(vk, /* flat */ true, is_null_free, is_atomic);
const TypeAryPtr* arytype = TypeOopPtr::make_from_klass(array_klass)->isa_aryptr();
arytype = arytype->cast_to_exactness(true);
arytype = arytype->cast_to_exactness(is_exact);
arytype = arytype->cast_to_not_null_free(is_not_null_free);
array = _gvn.transform(new CheckCastPPNode(control(), array, arytype, ConstraintCastNode::StrongDependency));
return array_element_address(array, idx, T_FLAT_ELEMENT, arytype->size(), control());
return _gvn.transform(new CastPPNode(control(), array, arytype, ConstraintCastNode::StrongDependency));
}

//-------------------------load_array_element-------------------------
Expand Down
3 changes: 1 addition & 2 deletions src/hotspot/share/opto/graphKit.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,8 +660,7 @@ class GraphKit : public Phase {
const TypeInt* sizetype = nullptr,
// Optional control dependency (for example, on range check)
Node* ctrl = nullptr);
Node* flat_array_element_address(Node*& array, Node* idx, ciInlineKlass* vk, bool is_null_free,
bool is_not_null_free, bool is_atomic);
Node* cast_to_flat_array(Node* array, ciInlineKlass* elem_vk, bool is_null_free, bool is_not_null_free, bool is_atomic);

// Return a load of array element at idx.
Node* load_array_element(Node* ary, Node* idx, const TypeAryPtr* arytype, bool set_ctrl);
Expand Down
85 changes: 33 additions & 52 deletions src/hotspot/share/opto/inlinetypenode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,19 +713,9 @@ void InlineTypeNode::store_flat_array(GraphKit* kit, Node* base, Node* idx) cons
DecoratorSet decorators = IN_HEAP | IS_ARRAY | MO_UNORDERED;
kit->C->set_flat_accesses();
ciInlineKlass* vk = inline_klass();
assert(vk->maybe_flat_in_array(), "");
assert(vk->maybe_flat_in_array(), "element type %s cannot be flat in array", vk->name()->as_utf8());

RegionNode* region;
if (vk->nof_nonstatic_fields() == 0) {
region = new RegionNode(3);
region->init_req(1, kit->top());
region->init_req(2, kit->top());
} else {
region = new RegionNode(4);
region->init_req(1, kit->top());
region->init_req(2, kit->top());
region->init_req(3, kit->top());
}
RegionNode* region = new RegionNode(4);
gvn.set_type(region, Type::CONTROL);
kit->record_for_igvn(region);

Expand All @@ -746,10 +736,10 @@ void InlineTypeNode::store_flat_array(GraphKit* kit, Node* base, Node* idx) cons
// Nullable
kit->set_control(kit->IfFalse(iff_null_free));
if (!kit->stopped()) {
assert(vk->has_nullable_atomic_layout(), "");
assert(vk->has_nullable_atomic_layout(), "element type %s does not have a nullable flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ false, /* not_null_free */ true, /* atomic */ true);
Node* cast = kit->cast_to_flat_array(base, vk, false, true, true);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);
if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr && vk->null_marker_offset_in_payload() != 0) {
// TODO 8357612 Weird layout
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why this would be specific to the layout described in JDK-8357612? Wouldn't we also hit vk->get_field_by_offset(vk->payload_offset(), false) == nullptr when the null marker is at vk->payload_offset() which would be totally fine?

Copy link
Member Author

Choose a reason for hiding this comment

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

The null marker at vk->payload_offset() is covered by vk->null_marker_offset_in_payload() == 0, am I right?

kit->insert_mem_bar(Op_MemBarCPUOrder);
Expand All @@ -760,31 +750,33 @@ void InlineTypeNode::store_flat_array(GraphKit* kit, Node* base, Node* idx) cons
store_flat(kit, cast, ptr, ptr_type, true, false, false, decorators);
}

region->set_req(1, kit->control());
region->init_req(1, kit->control());
mem->set_req(1, kit->reset_memory());
io->set_req(1, kit->i_o());
}

// Null-free
kit->set_control(kit->IfTrue(iff_null_free));
if (!kit->stopped()) {
kit->set_all_memory(input_memory_state);

if (vk->nof_nonstatic_fields() == 0) {
// Store into a null-free empty array is a nop. This short circuit must be done because we
// cannot calculate the address type of the elements in this case.
region->set_req(2, kit->control());
region->init_req(2, kit->control());
mem->set_req(2, kit->reset_memory());
io->set_req(2, kit->i_o());
} else {
Node* bol_atomic = kit->null_free_atomic_array_test(base, vk);
IfNode* iff_atomic = kit->create_and_map_if(kit->control(), bol_atomic, PROB_FAIR, COUNT_UNKNOWN);

// Atomic
kit->set_control(kit->IfTrue(iff_atomic));
if (!kit->stopped()) {
assert(vk->has_atomic_layout(), "");
assert(vk->has_atomic_layout(), "element type %s does not have a null-free atomic flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ true, /* not_null_free */ false, /* atomic */ true);
Node* cast = kit->cast_to_flat_array(base, vk, true, false, true);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);

if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr) {
// TODO 8357612 Weird layout
Expand All @@ -796,17 +788,18 @@ void InlineTypeNode::store_flat_array(GraphKit* kit, Node* base, Node* idx) cons
store_flat(kit, cast, ptr, ptr_type, true, false, true, decorators);
}

region->set_req(2, kit->control());
region->init_req(2, kit->control());
mem->set_req(2, kit->reset_memory());
io->set_req(2, kit->i_o());
}

// Non-atomic
kit->set_control(kit->IfFalse(iff_atomic));
if (!kit->stopped()) {
assert(vk->has_non_atomic_layout(), "");
assert(vk->has_non_atomic_layout(), "element type %s does not have a null-free non-atomic flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ true, /* not_null_free */ false, /* atomic */ false);
Node* cast = kit->cast_to_flat_array(base, vk, true, false, false);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);

if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr) {
// TODO 8357612 Weird layout
Expand All @@ -818,7 +811,7 @@ void InlineTypeNode::store_flat_array(GraphKit* kit, Node* base, Node* idx) cons
store_flat(kit, cast, ptr, ptr_type, false, false, true, decorators);
}

region->set_req(3, kit->control());
region->init_req(3, kit->control());
mem->set_req(3, kit->reset_memory());
io->set_req(3, kit->i_o());
}
Expand Down Expand Up @@ -1282,25 +1275,15 @@ InlineTypeNode* InlineTypeNode::make_from_flat_impl(GraphKit* kit, ciInlineKlass
}

InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlass* vk, Node* base, Node* idx) {
assert(vk->maybe_flat_in_array(), "");
assert(vk->maybe_flat_in_array(), "element type %s cannot be flat in array", vk->name()->as_utf8());
PhaseGVN& gvn = kit->gvn();
DecoratorSet decorators = IN_HEAP | IS_ARRAY | MO_UNORDERED | C2_CONTROL_DEPENDENT_LOAD;
kit->C->set_flat_accesses();
InlineTypeNode* vt_nullable = nullptr;
InlineTypeNode* vt_null_free = nullptr;
InlineTypeNode* vt_non_atomic = nullptr;

RegionNode* region;
if (vk->nof_nonstatic_fields() == 0) {
region = new RegionNode(3);
region->init_req(1, kit->top());
region->init_req(2, kit->top());
} else {
region = new RegionNode(4);
region->init_req(1, kit->top());
region->init_req(2, kit->top());
region->init_req(3, kit->top());
}
RegionNode* region = new RegionNode(4);
gvn.set_type(region, Type::CONTROL);
kit->record_for_igvn(region);

Expand All @@ -1321,10 +1304,10 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
// Nullable
kit->set_control(kit->IfFalse(iff_null_free));
if (!kit->stopped()) {
assert(vk->has_nullable_atomic_layout(), "");
assert(vk->has_nullable_atomic_layout(), "element type %s does not have a nullable flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ false, /* not_null_free */ true, /* atomic */ true);
Node* cast = kit->cast_to_flat_array(base, vk, false, true, true);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);
if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr && vk->null_marker_offset_in_payload() != 0) {
// TODO 8357612 Weird layout
kit->insert_mem_bar(Op_MemBarCPUOrder);
Expand All @@ -1335,7 +1318,7 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
vt_nullable = InlineTypeNode::make_from_flat(kit, vk, cast, ptr, ptr_type, true, false, false, decorators);
}

region->set_req(1, kit->control());
region->init_req(1, kit->control());
mem->set_req(1, kit->reset_memory());
io->set_req(1, kit->i_o());
}
Expand All @@ -1349,7 +1332,7 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
// Load from a null-free empty array, just return the default instance. This short circuit
// must be done because we cannot calculate the address type of the elements in this case.
vt_null_free = make_all_zero(gvn, vk);
region->set_req(2, kit->control());
region->init_req(2, kit->control());
mem->set_req(2, kit->reset_memory());
io->set_req(2, kit->i_o());
} else {
Expand All @@ -1359,10 +1342,10 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
// Atomic
kit->set_control(kit->IfTrue(iff_atomic));
if (!kit->stopped()) {
assert(vk->has_atomic_layout(), "");
assert(vk->has_atomic_layout(), "element type %s does not have a null-free atomic flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ true, /* not_null_free */ false, /* atomic */ true);
Node* cast = kit->cast_to_flat_array(base, vk, true, false, true);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);

if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr) {
// TODO 8357612 Weird layout
Expand All @@ -1374,20 +1357,18 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
vt_null_free = InlineTypeNode::make_from_flat(kit, vk, cast, ptr, ptr_type, true, false, true, decorators);
}

region->set_req(2, kit->control());
region->init_req(2, kit->control());
mem->set_req(2, kit->reset_memory());
io->set_req(2, kit->i_o());
}

// Non-Atomic
kit->set_control(kit->IfFalse(iff_atomic));
if (!kit->stopped()) {
assert(vk->has_non_atomic_layout(), "");
// TODO 8350865 Is the conversion to/from payload folded? We should wire this directly.
// Also remove the PreserveReexecuteState in Parse::array_load when buffering is no longer possible.
assert(vk->has_non_atomic_layout(), "element type %s does not have a null-free non-atomic flat layout", vk->name()->as_utf8());
kit->set_all_memory(input_memory_state);
Node* cast = base;
Node* ptr = kit->flat_array_element_address(cast, idx, vk, /* null_free */ true, /* not_null_free */ false, /* atomic */ false);
Node* cast = kit->cast_to_flat_array(base, vk, true, false, false);
Node* ptr = kit->array_element_address(cast, idx, T_FLAT_ELEMENT);

if (vk->get_field_by_offset(vk->payload_offset(), false) == nullptr) {
// TODO 8357612 Weird layout
Expand All @@ -1399,7 +1380,7 @@ InlineTypeNode* InlineTypeNode::make_from_flat_array(GraphKit* kit, ciInlineKlas
vt_non_atomic = InlineTypeNode::make_from_flat(kit, vk, cast, ptr, ptr_type, false, false, true, decorators);
}

region->set_req(3, kit->control());
region->init_req(3, kit->control());
mem->set_req(3, kit->reset_memory());
io->set_req(3, kit->i_o());
}
Expand Down
36 changes: 3 additions & 33 deletions src/hotspot/share/opto/parse2.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,26 +116,8 @@ void Parse::array_load(BasicType bt) {
if (!array_type->is_not_flat()) {
if (element_ptr->is_inlinetypeptr()) {
ciInlineKlass* vk = element_ptr->inline_klass();
bool is_null_free = array_type->is_null_free() || !vk->has_nullable_atomic_layout();
bool is_not_null_free = array_type->is_not_null_free() || (!vk->has_atomic_layout() && !vk->has_non_atomic_layout());
if (is_null_free) {
// TODO 8350865 Impossible type
is_not_null_free = false;
}
bool maybe_atomic = (!is_not_null_free && vk->has_atomic_layout()) || (!is_null_free && vk->has_nullable_atomic_layout());

ciArrayKlass* array_klass = ciArrayKlass::make(vk, /* flat */ true, is_null_free, maybe_atomic);
const TypeAryPtr* arytype = TypeOopPtr::make_from_klass(array_klass)->isa_aryptr();
arytype = arytype->cast_to_exactness(true);
arytype = arytype->cast_to_not_null_free(is_not_null_free);
Node* flat_array = gvn().transform(new CastPPNode(control(), array, arytype));

// Re-execute flat array load if buffering triggers deoptimization
PreserveReexecuteState preexecs(this);
jvms()->set_should_reexecute(true);
inc_sp(3);

Node* vt = InlineTypeNode::make_from_flat_array(this, element_ptr->inline_klass(), flat_array, array_index);
Node* flat_array = cast_to_flat_array(array, vk, false, false, false);
Node* vt = InlineTypeNode::make_from_flat_array(this, vk, flat_array, array_index);
ideal.set(res, vt);
} else {
// Element type is unknown, and thus we cannot statically determine the exact flat array layout. Emit a
Expand Down Expand Up @@ -285,19 +267,7 @@ void Parse::array_store(BasicType bt) {

if (vk != nullptr) {
// Element type is known, cast and store to flat array layout.
bool is_null_free = array_type->is_null_free() || !vk->has_nullable_atomic_layout();
bool is_not_null_free = array_type->is_not_null_free() || (!vk->has_atomic_layout() && !vk->has_non_atomic_layout());
if (is_null_free) {
// TODO 8350865 Impossible type
is_not_null_free = false;
}
bool maybe_atomic = (!is_not_null_free && vk->has_atomic_layout()) || (!is_null_free && vk->has_nullable_atomic_layout());

ciArrayKlass* array_klass = ciArrayKlass::make(vk, /* flat */ true, is_null_free, maybe_atomic);
const TypeAryPtr* arytype = TypeOopPtr::make_from_klass(array_klass)->isa_aryptr();
arytype = arytype->cast_to_exactness(true);
arytype = arytype->cast_to_not_null_free(is_not_null_free);
Node* flat_array = gvn().transform(new CastPPNode(control(), array, arytype));
Node* flat_array = cast_to_flat_array(array, vk, false, false, false);

// Re-execute flat array store if buffering triggers deoptimization
PreserveReexecuteState preexecs(this);
Expand Down