Skip to content

Commit

Permalink
More fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
TobiHartmann committed Jul 31, 2024
1 parent 570fab9 commit 1b24eac
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 34 deletions.
20 changes: 6 additions & 14 deletions src/hotspot/share/opto/inlinetypenode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,6 @@ void InlineTypeNode::make_scalar_in_safepoints(PhaseIterGVN* igvn, bool allow_oo
// If the inline type has a constant or loaded oop, use the oop instead of scalarization
// in the safepoint to avoid keeping field loads live just for the debug info.
Node* oop = get_oop();
// TODO 8325106
// TestBasicFunctionality::test3 fails without this. Add more tests?
// Add proj nodes here? Recursive handling of phis required? We need a test that fails without.
bool use_oop = false;
if (allow_oop && is_allocated(igvn) && oop->is_Phi()) {
Unique_Node_List worklist;
Expand All @@ -325,9 +322,6 @@ void InlineTypeNode::make_scalar_in_safepoints(PhaseIterGVN* igvn, bool allow_oo
Node* in = n->in(i);
if (in->is_Phi() && !visited.test_set(in->_idx)) {
worklist.push(in);
// TODO 8325106
// TestNullableArrays.test123 fails when enabling this, probably we should make sure that we don't load from a just allocated object
//} else if (!(in->is_Con() || in->is_Parm() || in->is_Load() || (in->isa_DecodeN() && in->in(1)->is_Load()))) {
} else if (!(in->is_Con() || in->is_Parm())) {
use_oop = false;
break;
Expand Down Expand Up @@ -377,7 +371,7 @@ void InlineTypeNode::make_scalar_in_safepoints(PhaseIterGVN* igvn, bool allow_oo
vt->make_scalar_in_safepoints(igvn);
}
if (outcnt() == 0) {
igvn->_worklist.push(this);
igvn->record_for_igvn(this);
}
}

Expand Down Expand Up @@ -722,8 +716,8 @@ static void replace_allocation(PhaseIterGVN* igvn, Node* res, Node* dom) {

Node* InlineTypeNode::Ideal(PhaseGVN* phase, bool can_reshape) {
Node* oop = get_oop();
const Type* tinit = phase->type(get_is_init());
if ((tinit->isa_int() && tinit->is_int()->is_con(1)) &&
const TypeInt* tinit = phase->type(get_is_init())->isa_int();
if ((tinit != nullptr && tinit->is_con(1)) &&
((is_default(phase) && !is_larval(phase) && !is_larval()) || inline_klass()->is_empty()) &&
inline_klass()->is_initialized() &&
(!oop->is_Con() || phase->type(oop)->is_zero_type())) {
Expand Down Expand Up @@ -1042,9 +1036,8 @@ Node* InlineTypeNode::is_loaded(PhaseGVN* phase, ciInlineKlass* vk, Node* base,
vk = inline_klass();
}
if (field_count() == 0 && vk->is_initialized()) {
const Type* tinit = phase->type(in(IsInit));
// TODO 8325106
if (false && !is_larval() && tinit->isa_int() && tinit->is_int()->is_con(1)) {
const TypeInt* tinit = phase->type(get_is_init())->isa_int();
if (tinit != nullptr && tinit->is_con(1)) {
assert(is_allocated(phase), "must be allocated");
return get_oop();
} else {
Expand Down Expand Up @@ -1243,8 +1236,7 @@ void InlineTypeNode::remove_redundant_allocations(PhaseIdealLoop* phase) {
if (res == nullptr || !res->is_CheckCastPP()) {
break; // No unique CheckCastPP
}
// TODO 8325106
// assert((!is_default(igvn) || !inline_klass()->is_initialized()) && !is_allocated(igvn), "re-allocation should be removed by Ideal transformation");
assert((!is_default(igvn) || !inline_klass()->is_initialized()) && !is_allocated(igvn), "re-allocation should be removed by Ideal transformation");
// Search for a dominating allocation of the same inline type
Node* res_dom = res;
for (DUIterator_Fast jmax, j = fast_outs(jmax); j < jmax; j++) {
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/inlinetypenode.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class InlineTypeNode : public TypeNode {
bool _is_larval;

virtual uint hash() const { return TypeNode::hash() + _is_larval; }
// Don't GVN larvals during parsing because the inputs might be updated
virtual bool cmp(const Node &n) const { return TypeNode::cmp(n) && (Compile::current()->scalarize_in_safepoints() || !(n.isa_InlineType()->_is_larval || _is_larval)); }
// Don't GVN larvals because the inputs might be updated
virtual bool cmp(const Node &n) const { return TypeNode::cmp(n) && !(n.isa_InlineType()->_is_larval || _is_larval); }
virtual uint size_of() const { return sizeof(*this); }

// Get the klass defining the field layout of the inline type
Expand Down
1 change: 1 addition & 0 deletions src/hotspot/share/opto/macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -690,6 +690,7 @@ bool PhaseMacroExpand::can_eliminate_allocation(PhaseIterGVN* igvn, AllocateNode
NOT_PRODUCT(fail_eliminate = "null or TOP memory";)
can_eliminate = false;
} else if (!reduce_merge_precheck) {
assert(!res->is_Phi() || !res->as_Phi()->can_be_inline_type(), "Inline type allocations should not have safepoint uses");
safepoints->append_if_missing(sfpt);
}
} else if (use->is_InlineType() && use->as_InlineType()->get_oop() == res) {
Expand Down
26 changes: 9 additions & 17 deletions src/hotspot/share/opto/parse3.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -328,34 +328,26 @@ void Parse::set_inline_type_field(Node* obj, ciField* field, Node* val) {
assert(obj->as_InlineType()->is_larval(), "must be larval");
assert(!_gvn.type(obj)->maybe_null(), "should never be null");

// Re-execute if buffering in below code triggers deoptimization.
PreserveReexecuteState preexecs(this);
jvms()->set_should_reexecute(true);
inc_sp(1);

if (!val->is_InlineType() && field->type()->is_inlinetype()) {
// Scalarize inline type field value
val = InlineTypeNode::make_from_oop(this, val, field->type()->as_inline_klass(), field->is_null_free());
} else if (val->is_InlineType() && !field->is_flat()) {
// TODO 8325106 Why do we need to buffer here when the field type is an inline type?
// Field value needs to be allocated because it can be merged with an oop.
// Re-execute if buffering triggers deoptimization.
PreserveReexecuteState preexecs(this);
jvms()->set_should_reexecute(true);
inc_sp(1);
// Field value needs to be allocated because it can be merged with a non-inline type.
val = val->as_InlineType()->buffer(this);
}

// Clone the inline type node and set the new field value
InlineTypeNode* new_vt = obj->as_InlineType()->clone_if_required(&_gvn, _map);
new_vt->set_field_value_by_offset(field->offset_in_bytes(), val);
{
PreserveReexecuteState preexecs(this);
jvms()->set_should_reexecute(true);
inc_sp(1);
new_vt = new_vt->adjust_scalarization_depth(this);
}
new_vt = new_vt->adjust_scalarization_depth(this);

// TODO 8325106 Double check and explain these checks
if ((!_caller->has_method() || C->inlining_incrementally() || _caller->method()->is_object_constructor()) && new_vt->is_allocated(&gvn())) {
assert(new_vt->as_InlineType()->is_allocated(&gvn()), "must be buffered");
// We need to store to the buffer
// TODO 8325106 looks like G1BarrierSetC2::g1_can_remove_pre_barrier is not strong enough to remove the pre barrier
// If the inline type is buffered and the caller might use the buffer, update it.
if (new_vt->is_allocated(&gvn()) && (!_caller->has_method() || C->inlining_incrementally() || _caller->method()->is_object_constructor())) {
new_vt->store(this, new_vt->get_oop(), new_vt->get_oop(), new_vt->bottom_type()->inline_klass(), 0, field->offset_in_bytes());

// Preserve allocation ptr to create precedent edge to it in membar
Expand Down
1 change: 0 additions & 1 deletion src/hotspot/share/opto/phaseX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1511,7 +1511,6 @@ void PhaseIterGVN::add_users_of_use_to_worklist(Node* n, Node* use, Unique_Node_
}

// AndLNode::Ideal folds GraphKit::mark_word_test patterns. Give it a chance to run.
// TODO 8325106 Improve this to handle all patterns
if (n->is_Load() && use->is_Phi()) {
for (DUIterator_Fast imax, i = use->fast_outs(imax); i < imax; i++) {
Node* u = use->fast_out(i);
Expand Down

0 comments on commit 1b24eac

Please sign in to comment.