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

[Vladimir Ivanov]

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

Yes, it should be I1511. Sorry for the confusion.

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

PhaseIdealLoop::Dominators() doesn't adjust IDOM for Regions. So, 
initial IDOM values are and that's the same dom_lca_internal() computes 
for them:
   IDOM(R1710) = IDOM(R1716) = IDOM(R1722) = I1511

Then IdealLoopTree::counted_loop() strip mines some of the loops and it 
causes a change in R1722 which causes recomputation of IDOM using 
dom_lca() which does normalize the IDOM.

If IDOM is rebuilt from scratch at this point, initial IDOM will stay 
the same (because no strip mining takes place):
   IDOM(R1710) = IDOM(R1716) = IDOM(R1722) = I1511


And that's the other way to fix the crash: initiate new PhaseIdealLoop 
iteration right away if any strip mined loops are introduced.

But it looks more like a workaround and I decided to go with the fix in 
PhaseIdealLoop::spinup() because I don't see a reason why IDOM 
recomputation can't be triggered from other places.

Best regards,
Vladimir Ivanov