[12] RFR(S): 8208275: C2 crash in Node::add_req(Node*)

[Roland Westrelin]

Hi Tobias,

> A loop with only one iteration is strip mined and then processed by
> IdealLoopTree::policy_do_one_iteration_loop() which replaces the tripcount phi such that the
> backedge becomes dead and the loop goes away. Usually, the code in RegionNode::Ideal [1] takes care
> of removing the OuterStripMinedLoop as well but in the failing case the split-if optimization is
> invoked first (it depends on the order in which igvn processes nodes in the worklist). Split-if now
> performs its magic and then removes the CountedLoop without removing the OuterStripMinedLoop [2].

So an if is split through a counted loop head?
In general, that would break loop strip mining because it would cause
some control flow to be inserted between the OuterStripMinedLoop and the
new loop heads. And I think that's the root of the problem. 

> The straight forward fix is to remove the outer loop in policy_do_one_iteration_loop() similar to
> what we do in PhaseIdealLoop::intrinsify_fill(). I was wondering if other optimizations could
> trigger the same problem but came to the conclusion that split-if would bail out because either:
> - the tripcount phi was removed and the counted loop backedge is already top [3] or
> - the tripcount phi is still there and used by the AddI for increment [4]

That would not fix the root cause. It would fix the case where the
counted loop is going away. But applying split if to a counted loop when
the counted loop is not being optimized away would break.

> An alternative would be to bail out of the split-if optimization if the region is a strip mined loop
> or remove the outer strip mined loop if the region is removed.

Removing the outer strip mined loop entirely is not ok. There could
still be loops after split if and that would make those loops run with
no safepoint at all.

Bailing out would be fine if that code never triggers with counted loops
but the code has some tests for CountedLoop so it looks like it does
(but it's strange that this never broke before).

I would say the best way to deal with this would be to transform the
outer strip mined loop + counted loop back into the initial counted loop
with a safepoint in the loop and then apply split if. Further loop opts
passes would have a chance to build strip mined loop nests for the
remaining loops.

The loop nest structure is:

OuterStripMinedLoop
CountedLoop
CountedLoopEnd
SafePoint
If // branches back to OuterStripMinedLoop

and we would to turn this into:

Region
SafePoint
If // branches back to OuterStripMinedLoop

by:

removing the OuterStripMinedLoop
cloning the CountedLoop into a Region with the backedge of the
OuterStripMinedLoop
removing the CountedLoopEnd
Changing the If condition to that of the CountedLoopEnd

Roland.