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

[Vladimir Kozlov]

On 1/14/19 1:55 PM, Vladimir Ivanov wrote:
> 
>>>> R1722 is changed during strip mining transformation and its IDOM is recomputed (I1511 => 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.

Got it. So the issue is that strip mining invalidated IDOM information generated at the beginning of 
PhaseIdealLoop::build_and_optimize().

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

I am not sure your changes help to all cases.  It may indeed helps to split_if optimization but dominator information is 
used before it too. I see Shenandoah's optimize_loops() uses information before split_if.

Can we correctly recalculate IDOM after counted_loop() if strip mining loop was inserted? My be we can simplify strip 
mining code if we know that IDOM will be recalculated.

Would be nice to hear Roland's opinion too.


On other hand I think your point fix is good for JDK 12. May be do what I suggest in JDK 13 later if it is too complex.

Thanks,
Vladimir

> 
> Best regards,
> Vladimir Ivanov