Skip to content
Permalink
Browse files

8241997: Scalar replacement of cloned array is broken after JDK-8238759

Replacement code still expected ArrayCopyNode::Dest to be an AddPNode.

Reviewed-by: roland, neliasso
  • Loading branch information
TobiHartmann committed Apr 6, 2020
1 parent 048c5c0 commit 3d36ef14b33d67c665889961eff860426f2bc3e7
Showing with 43 additions and 49 deletions.
  1. +38 −42 src/hotspot/share/opto/macro.cpp
  2. +5 −7 src/hotspot/share/opto/memnode.cpp
@@ -364,7 +364,7 @@ Node* PhaseMacroExpand::make_arraycopy_load(ArrayCopyNode* ac, intptr_t offset,
Node* res = NULL;
if (ac->is_clonebasic()) {
assert(ac->in(ArrayCopyNode::Src) != ac->in(ArrayCopyNode::Dest), "clone source equals destination");
Node* base = ac->in(ArrayCopyNode::Src)->in(AddPNode::Base);
Node* base = ac->in(ArrayCopyNode::Src);
Node* adr = _igvn.transform(new AddPNode(base, base, MakeConX(offset)));
const TypePtr* adr_type = _igvn.type(base)->is_ptr()->add_offset(offset);
res = LoadNode::make(_igvn, ctl, mem, adr, adr_type, type, bt, MemNode::unordered, LoadNode::UnknownControl);
@@ -671,11 +671,8 @@ bool PhaseMacroExpand::can_eliminate_allocation(AllocateNode *alloc, GrowableArr
for (DUIterator_Fast kmax, k = use->fast_outs(kmax);
k < kmax && can_eliminate; k++) {
Node* n = use->fast_out(k);
if (!n->is_Store() && n->Opcode() != Op_CastP2X &&
SHENANDOAHGC_ONLY((!UseShenandoahGC || !ShenandoahBarrierSetC2::is_shenandoah_wb_pre_call(n)) &&)
!(n->is_ArrayCopy() &&
n->as_ArrayCopy()->is_clonebasic() &&
n->in(ArrayCopyNode::Dest) == use)) {
if (!n->is_Store() && n->Opcode() != Op_CastP2X
SHENANDOAHGC_ONLY(&& (!UseShenandoahGC || !ShenandoahBarrierSetC2::is_shenandoah_wb_pre_call(n))) ) {
DEBUG_ONLY(disq_node = n;)
if (n->is_Load() || n->is_LoadStore()) {
NOT_PRODUCT(fail_eliminate = "Field load";)
@@ -686,7 +683,8 @@ bool PhaseMacroExpand::can_eliminate_allocation(AllocateNode *alloc, GrowableArr
}
}
} else if (use->is_ArrayCopy() &&
(use->as_ArrayCopy()->is_arraycopy_validated() ||
(use->as_ArrayCopy()->is_clonebasic() ||
use->as_ArrayCopy()->is_arraycopy_validated() ||
use->as_ArrayCopy()->is_copyof_validated() ||
use->as_ArrayCopy()->is_copyofrange_validated()) &&
use->in(ArrayCopyNode::Dest) == res) {
@@ -967,18 +965,6 @@ void PhaseMacroExpand::process_users_of_allocation(CallNode *alloc) {
}
#endif
_igvn.replace_node(n, n->in(MemNode::Memory));
} else if (n->is_ArrayCopy()) {
// Disconnect ArrayCopy node
ArrayCopyNode* ac = n->as_ArrayCopy();
assert(ac->is_clonebasic(), "unexpected array copy kind");
Node* membar_after = ac->proj_out(TypeFunc::Control)->unique_ctrl_out();
disconnect_projections(ac, _igvn);
assert(alloc->in(0)->is_Proj() && alloc->in(0)->in(0)->Opcode() == Op_MemBarCPUOrder, "mem barrier expected before allocation");
Node* membar_before = alloc->in(0)->in(0);
disconnect_projections(membar_before->as_MemBar(), _igvn);
if (membar_after->is_MemBar()) {
disconnect_projections(membar_after->as_MemBar(), _igvn);
}
} else {
eliminate_gc_barrier(n);
}
@@ -988,30 +974,40 @@ void PhaseMacroExpand::process_users_of_allocation(CallNode *alloc) {
} else if (use->is_ArrayCopy()) {
// Disconnect ArrayCopy node
ArrayCopyNode* ac = use->as_ArrayCopy();
assert(ac->is_arraycopy_validated() ||
ac->is_copyof_validated() ||
ac->is_copyofrange_validated(), "unsupported");
CallProjections callprojs;
ac->extract_projections(&callprojs, true);

_igvn.replace_node(callprojs.fallthrough_ioproj, ac->in(TypeFunc::I_O));
_igvn.replace_node(callprojs.fallthrough_memproj, ac->in(TypeFunc::Memory));
_igvn.replace_node(callprojs.fallthrough_catchproj, ac->in(TypeFunc::Control));

// Set control to top. IGVN will remove the remaining projections
ac->set_req(0, top());
ac->replace_edge(res, top());

// Disconnect src right away: it can help find new
// opportunities for allocation elimination
Node* src = ac->in(ArrayCopyNode::Src);
ac->replace_edge(src, top());
// src can be top at this point if src and dest of the
// arraycopy were the same
if (src->outcnt() == 0 && !src->is_top()) {
_igvn.remove_dead_node(src);
if (ac->is_clonebasic()) {
Node* membar_after = ac->proj_out(TypeFunc::Control)->unique_ctrl_out();
disconnect_projections(ac, _igvn);
assert(alloc->in(0)->is_Proj() && alloc->in(0)->in(0)->Opcode() == Op_MemBarCPUOrder, "mem barrier expected before allocation");
Node* membar_before = alloc->in(0)->in(0);
disconnect_projections(membar_before->as_MemBar(), _igvn);
if (membar_after->is_MemBar()) {
disconnect_projections(membar_after->as_MemBar(), _igvn);
}
} else {
assert(ac->is_arraycopy_validated() ||
ac->is_copyof_validated() ||
ac->is_copyofrange_validated(), "unsupported");
CallProjections callprojs;
ac->extract_projections(&callprojs, true);

_igvn.replace_node(callprojs.fallthrough_ioproj, ac->in(TypeFunc::I_O));
_igvn.replace_node(callprojs.fallthrough_memproj, ac->in(TypeFunc::Memory));
_igvn.replace_node(callprojs.fallthrough_catchproj, ac->in(TypeFunc::Control));

// Set control to top. IGVN will remove the remaining projections
ac->set_req(0, top());
ac->replace_edge(res, top());

// Disconnect src right away: it can help find new
// opportunities for allocation elimination
Node* src = ac->in(ArrayCopyNode::Src);
ac->replace_edge(src, top());
// src can be top at this point if src and dest of the
// arraycopy were the same
if (src->outcnt() == 0 && !src->is_top()) {
_igvn.remove_dead_node(src);
}
}

_igvn._worklist.push(ac);
} else {
eliminate_gc_barrier(use);
@@ -538,8 +538,7 @@ Node* LoadNode::find_previous_arraycopy(PhaseTransform* phase, Node* ld_alloc, N
mb->in(0)->in(0) != NULL && mb->in(0)->in(0)->is_ArrayCopy()) {
ArrayCopyNode* ac = mb->in(0)->in(0)->as_ArrayCopy();
if (ac->is_clonebasic()) {
intptr_t offset;
AllocateNode* alloc = AllocateNode::Ideal_allocation(ac->in(ArrayCopyNode::Dest), phase, offset);
AllocateNode* alloc = AllocateNode::Ideal_allocation(ac->in(ArrayCopyNode::Dest), phase);
if (alloc != NULL && alloc == ld_alloc) {
return ac;
}
@@ -946,12 +945,11 @@ Node* LoadNode::can_see_arraycopy_value(Node* st, PhaseGVN* phase) const {
if (ac->as_ArrayCopy()->is_clonebasic()) {
assert(ld_alloc != NULL, "need an alloc");
assert(addp->is_AddP(), "address must be addp");
assert(ac->in(ArrayCopyNode::Dest)->is_AddP(), "dest must be an address");
BarrierSetC2* bs = BarrierSet::barrier_set()->barrier_set_c2();
assert(bs->step_over_gc_barrier(addp->in(AddPNode::Base)) == bs->step_over_gc_barrier(ac->in(ArrayCopyNode::Dest)->in(AddPNode::Base)), "strange pattern");
assert(bs->step_over_gc_barrier(addp->in(AddPNode::Address)) == bs->step_over_gc_barrier(ac->in(ArrayCopyNode::Dest)->in(AddPNode::Address)), "strange pattern");
addp->set_req(AddPNode::Base, src->in(AddPNode::Base));
addp->set_req(AddPNode::Address, src->in(AddPNode::Address));
assert(bs->step_over_gc_barrier(addp->in(AddPNode::Base)) == bs->step_over_gc_barrier(ac->in(ArrayCopyNode::Dest)), "strange pattern");
assert(bs->step_over_gc_barrier(addp->in(AddPNode::Address)) == bs->step_over_gc_barrier(ac->in(ArrayCopyNode::Dest)), "strange pattern");
addp->set_req(AddPNode::Base, src);
addp->set_req(AddPNode::Address, src);
} else {
assert(ac->as_ArrayCopy()->is_arraycopy_validated() ||
ac->as_ArrayCopy()->is_copyof_validated() ||

0 comments on commit 3d36ef1

Please sign in to comment.