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

[Vladimir Kozlov]

On 1/11/19 11:23 AM, Vladimir Ivanov wrote:
> 
> 
> 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)

Is it typo? Should it be I1511? I don't see I1522 in graph's picture.

>      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

I assume it is 1511.

>      find_non_split_ctrl(I1522) = R1784
> 
> If IDOM info is recomputed from scratch, IDOM(R1722) remains I1511.

Can you explain more this point? Why result is different if it is from scratch?

Thanks,
Vladimir K

> 
> 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