RFR (S): 8215757: C2: PhaseIdealLoop::spinup() computes wrong post-dominating point
vladimir.x.ivanov at oracle.com
Mon Jan 14 23:49:15 UTC 2019
>> 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.
I try to address only PhaseIdealLoop::spinup() case. There may be other
bugs lurking in other places.
> 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.
The simplest fix I can come up with (and most reliable IMO w.r.t. other
possible bugs which aren't uncovered yet) is to set C->major_progress()
if strip mining happened and return early to initiate the next round of
PhaseIdealLoop and recompute IDOM info. In that case, transformations
will see only IDOM computed by Dominators(), but it means repeated IDOM
& loop info computations when strip mining happens.
> Would be nice to hear Roland's opinion too.
Yes, same here.
As for me:
* I find it ugly that Dominators() and dom_lca() aren't consistent;
* I'm in favor of normalized info (dom_lca() variant) to be computed
from the very beginning;
* I still believe PhaseIdealLoop::spinup() has a bug which should be
fixed (irrespective of whether IDOM is normalized or not);
More information about the hotspot-compiler-dev