RFR [9] (XS): 8179882: C2: Stale control info after cast node elimination during loop optimization pass

[Vladimir Ivanov]

>>>> The problem with it is that ConstraintCastNode::dominating_cast()
>>>> relies on immediate control info (in(0)) which can get out of sync
>>>> with what is cached in PhaseIdealLoop (get_ctrl()).
>>>
>>> Should we fix this instead? It means somewhere we forgot to update loop
>>> info.
>>
>> Can you elaborate your point, please? The fix being proposed does
>> exactly that - detect the case when dom_cast should be moved and
>> updates its control & loop info to dominate all users
>> (dom_lca(dom_cast_ctrl, n_ctrl)).
>
> Could be misunderstanding. You said get_ctrl() does not point to in(0)
> at time when dominating_cast() is called. My question is why? During
> loop construction it should point to it. At which time it is changed?

Step-by-step description of applied transformations follows (details [1]):
   #1: If: 1012 => 1257
      - same condition (1820 Bool), 1257 dominates 1012

   #2: CastPP: 5846 => 733
      - 733 dominates 5846

   #3: CheckCastPP: 773 => 574
      - 773 & 574 have the same control node after #1 (1260/1257), but 
get_ctrl(dom_cast) points to 1581 (before #2: 574 -1-> 5846 -0-> 1581).

The problem is that 773 was part of N4442 and 574 doesn't dominate the 
loop body, but #3 doesn't place 574 there when replacing 773. So, 
subsequent cloning of loop body during loop peeling of N4442 produces 
unschedulable graph.

The fix moves 574 up the dominator tree and puts it inside N4442, so it 
dominates all users of 773 & 574 after #3.

get_ctrl() & in(0) point to different CFG nodes after #1, but the loop 
info is correct until #3 happens which requires 574 to be moved up.

Best regards,
Vladimir Ivanov

[1]
=== new loop opt pass

#0: Initial state
   Loop: N4442/N1838  body={ 1257 773 1260 }
   Loop: N4445/N2197  body={ 1012 574 1075 }

  574    CheckCastPP    ===  1075  5846 
Oop:java/lang/Integer:NotNull:exact *
      1075    IfFalse    ===  1012
          1012    If    ===  1575  1820
              1820    Bool    ===  ...
      5846    CastPP    ===  1581  677
          1581    Proj    ===  2137 #0  Type:control
              2137    Unlock    === ...
          677    Phi    === ...

  773    CheckCastPP    ===  1260  733 
Oop:java/lang/Integer:NotNull:exact *
      1260    IfFalse    ===  1257
          1257    If    ===  1819  1820
              1820    Bool    === ...
      733    CastPP    ===  1230  677
          1230    IfTrue    ===  1225
              1225    If    === ...
          677    Phi    === ...


#1: 1012 => 1257 (same condition, 1257 dominates 1012) and 574 isn't 
part of N4445 anymore

   Loop: N4442/N1838  body={ 1257 773 1260 }
   Loop: N4445/N2197  body={ }

  574    CheckCastPP    ===  1260  5846
      1260    IfFalse    ===  1257
          1257    If    ===  1575  1820
              1820    Bool    ===  ...
      5846    CastPP    ===  1581  677
          1581    Proj    ===  2137 #0  Type:control
              2137    Unlock    === ...
          677    Phi    === ...

  773    CheckCastPP    ===  1260  733
      1260    IfFalse    ===  1257
          1257    If    ===  1819  1820
              1820    Bool    === ...
      733    CastPP    ===  1230  677
          1230    IfTrue    ===  1225
              1225    If    === ...
          677    Phi    === ...

=== next loop opts pass

   Loop: N4442/N1838  body={ 1257 773 1260 }
   Loop: N4445/N2197  body={ }

#2: 5846 => 733

  574    CheckCastPP    ===  1260  733
  773    CheckCastPP    ===  1260  733

      1260    IfFalse    ===  1257
          1257    If    ===  1819  1820
              1820    Bool    === ...

      733    CastPP    ===  1230  677
          1230    IfTrue    ===  1225
              1225    If    === ...
          677    Phi    === ...

   Loop: N4442/N1838  body={ 1257 773 1260 }
   Loop: N4445/N2197  body={ }

#3: 773 => 574

   Loop: N4442/N1838  body={ 1257 1260 } // NB! missing 574
   Loop: N4445/N2197  body={ }

#4: Peel N4442 (NB! same loop opt iteration, so loop tree hasn't been 
rebuilt yet.)
   - 574 isn't cloned since it's not part of the loop body and it leads 
to unscheduleable IR.

=== Crash

> If there was a new node created during loopopts then
> PhaseIdealLoop::register_new_node() should be called for it already.
>
> Vladimir K
>
>>
>> Or do you suggest to rewrite ConstraintCastNode::dominating_cast() to
>> use get_ctrl() instead of in(0)?
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>>>
>>>> The fix is to catch the case when dom_cast doesn't dominate n based on
>>>> info from PhaseIdealLoop and update control info accordingly.
>>>>
>>>> Testing: manual (replayed problematic compilation & eyeballed the IR),
>>>> JPRT, RBT (hs-tier0-comp, in progress).
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> PS: thanks to Roland for helping with the fix.