RFR(M): 8212610: Fix handling of memory in PhaseIdealLoop::clone_loop_predicates()

[Vladimir Kozlov]

Thank you for explanation, Roland. I understand why new Phi node is needed.

One thing left is may be use r->find_edge(other_proj) instead of r->req()-1 in igvn->replace_input_of(phi,
I known that rgn->add_req(if_uct); is used for new clone but I don't want any surprises if the edge is not last for some 
reasons.

Thanks,
Vladimir

On 10/23/18 8:49 AM, Roland Westrelin wrote:
> 
> We found a bug in that code so here is an updated webrev:
> 
> http://cr.openjdk.java.net/~roland/8212610/webrev.01/
> 
>> In what case we will not have memory Phi when there is region for cloned predicate? I thought it was always created. And
>> your new created Phi has the same input (mem) on all branches - would not it be optimized out later?
> 
> With this before cloning:
> 
> Predicate 1
> ..
> Predicate n
> 
> profile predicate 1
> ..
> write barrier
> ...
> profile predicate n
> 
> loop head
> 
> There are 2 uncommon trap calls (one for predicates, one for profile
> predicates), each one is dominated by a region. In the example above,
> the region for predicates has no phi because there's no memory producing
> nodes that's control dependent on one of the predicates. The region for
> profile predicates has a phi to merge memory state before and after the
> write barrier. Then:
> 
> Predicate 1
> ..
> Predicate n
> 
> profile predicate 1
> ..
> write barrier
> ...
> profile predicate n
> 
> cloned predicate
> 
> cloned profile predicate
> 
> the region for predicates still has no phi but the cloned predicates is
> dominated by a write barrier so the control edge that's coming into the
> region for predicates from the cloned predicate has different memory
> state and a memory phi is needed.
> 
> One of the inputs of the newly created phi is updated by:
> 
> 364         igvn->replace_input_of(phi, r->req()-1, dom_use->in(dom_r->find_edge(other_dom_proj)));
> 
> so the phi doesn't have identical inputs.
> 
>> Why you removed is_Proj() from assert() in second part of changes?
> 
> PhaseIdealLoop::clone_predicate() returns a ProjNode so the is_Proj() is
> useless.
> 
> Roland.
>