RFR [9] (XS): 8179882: C2: Stale control info after cast node elimination during loop optimization pass
Vladimir Kozlov
vladimir.kozlov at oracle.com
Thu May 25 01:48:11 UTC 2017
On 5/24/17 5:50 PM, Vladimir Ivanov wrote:
>
>
> On 5/25/17 3:24 AM, Vladimir Kozlov wrote:
>> On 5/24/17 4:38 PM, Vladimir Ivanov wrote:
>>>>>>> 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
>>
>> Which transformation replace 1012 with 1257? We should exit loop opts
>> after such transformation is done and recalculate new loop info again on
>> next loop opts iteration.
>
> Sorry for the confusion. #1 & #2-#3 happen in different (consequent)
> loop opts iterations. Loop info is rebuilt after #2 and it goes out of
> sync during #2-#3.
You mean: rebuild after #1. Right?
>
>> Based on code it look like we are doing split_if_with_blocks when this
>> happens. major_progress should be set and exit this iteration of loop
>> opts. So why "loop cloning" happens?
>
> Cast node elimination in split_if_with_blocks_pre doesn't bump
> _major_progress, so after split-ifs are over (#2-#3 are part of them),
> the compiler proceeds to loop opts.
Rereading this part:
>>> 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,
Would it help if 574 CheckCastPP is replaced with 773?
Vladimir
>
> Calling set_major_progress() when dom_cast doesn't dominate n in
> split_if_with_blocks_pre() is another way to fix the bug, but it is much
> more expensive (introduces additional loop opts pass).
>
> Best regards,
> Vladimir Ivanov
>
>>>
>>> #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.
More information about the hotspot-compiler-dev
mailing list