[12] RFR (S): 8215757: C2: PhaseIdealLoop::spinup() computes wrong post-dominating point

[Vladimir Ivanov]

On 11/01/2019 10:23, Vladimir Ivanov wrote:
> 
>>>     #2: As part of loop limit check insertion, new IfNode is created (If
>>> 1854) and linked to R1722 as an input which causes R1722 IDOM to be
>>> updated [2]. It changes R1722 IDOM (I1511 => R1784), since dom_lca()
>>> normalizes the result using find_non_split_ctrl().
>>
>> Isn't that the root cause: the idom of R1722 is still I1511 and not
>> R1784?
> 
> If it were the case, then PhaseIdealLoop::handle_use()/spinup() would 
> reliably crash on all users of Phi 1790. There are 2 other Regions 
> (R1710 and R1716) which keep their IDOM (I1511) intact and the 
> transformation works fine for them.
> 
> R1722 is changed during strip mining transformation and its IDOM is 
> recomputed (I1511 => R1784). 

To elaborate a bit more on that: the only reason IDOM changes is due to 
the way it is computed:
     // rgn = R1722, new_iff = I1854
     Node* ridom = idom(rgn); // ridom = I1522 = IDOM(R1722)
     Node* nrdom = dom_lca(ridom, new_iff); // nrdom = R1784
     set_idom(rgn, nrdom, dom_depth(rgn));

     Node *dom_lca( Node *n1, Node *n2 ) const {
       return find_non_split_ctrl(dom_lca_internal(n1, n2));
     }

     dom_lca_internal(I1522, I1854) = I1522
     find_non_split_ctrl(I1522) = R1784

If IDOM info is recomputed from scratch, IDOM(R1722) remains I1511.

So, eager IDOM normalization (during initial construcion) doesn't help: 
it would lead to consistently hitting the problem in 
PhaseIdealLoop::handle_use()/spinup() when processing dependent RegionNodes.

Best regards,
Vladimir Ivanov