[13] RFR (XS): 8202952: C2: Unexpected dead nodes after matching

[Vladimir Ivanov]

Thanks, Vladimir.

Best regards,
Vladimir Ivanov

On 22/01/2019 12:42, Vladimir Kozlov wrote:
> Got it. Good.
> 
> thanks,
> Vladimir
> 
> On 1/22/19 12:08 PM, Vladimir Ivanov wrote:
>>
>> On 22/01/2019 11:54, Vladimir Kozlov wrote:
>>> The fix is different from what we discussed.
>>> Can you explain how it helps?
>>
>> We discussed adding AddP case to _shared_nodes.
>>
>> Proposed fix achieves similar result with a different approach:
>>
>>    * Matcher::clone_address_expressions() marks problematic AddP as 
>> shared (based on constant value);
>>
>>    * DFA() doesn't construct duplicated State for inner AddP (since 
>> it's marked as shared);
>>
>>    * Matcher doesn't need to materialize duplicated mach nodes, since 
>> it matches inner AddP separately;
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>> On 1/22/19 11:05 AM, Vladimir Ivanov wrote:
>>>> http://cr.openjdk.java.net/~vlivanov/8202952/webrev.00/
>>>> https://bugs.openjdk.java.net/browse/JDK-8202952
>>>>
>>>> The crash happens when PhaseCFG encounters a dead MachNode in the 
>>>> graph.
>>>> The problematic node is a leftover from matching of an instruction 
>>>> with a duplicated memory operand (sarI_mem_CL [1] in that particular 
>>>> case).
>>>>
>>>> Address has the following shape [2]:
>>>>    AddP (AddP DecodeN (LShiftL ConvI2L ConI)) ConL
>>>>
>>>> It could be subsumed into complex addressing expression, but the 
>>>> constant is too large (doesn't fit into immL32). So, matcher has to 
>>>> compute inner address expression separately and put it into a register.
>>>>
>>>> Since memory operand is duplicated, 2 copies are materialized during 
>>>> matching, but as part of ::Expand() one of the copies is eliminated, 
>>>> thus leaving a dead mach node in the IR (for the address expression).
>>>>
>>>> The fix is to adjust Matcher::clone_address_expressions() to avoid 
>>>> cloning inner AddP when constant offset is too large.
>>>>
>>>> Testing: hs-precheckin-comp, hs-tier1, hs-tier2
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>> [1] instruct sarI_mem_CL(memory dst, rcx_RegI shift, rFlagsReg cr)
>>>> %{
>>>>    match(Set dst (StoreI dst (RShiftI (LoadI dst) shift)));
>>>>
>>>>
>>>> [2]
>>>>   o347 AddP  === _ o2181 o1768 o1769  [[o349 o371 ]]
>>>>      o1768 AddP  === _ o2181 o2181 o1765  [[o347 ]]
>>>>          o2181 DecodeN === _ o287  [[o1768 o1768 o327 o347 o327 ]] 
>>>> #int[int:>=0]:NotNull:exact *
>>>>          o1765 LShiftL === _ o1761 o60  [[o1768 ]]
>>>>              o1761 ConvI2L === _ o1741  [[o1765 ]] 
>>>> #long:maxint-51..maxint-48
>>>>              o60   ConI  === o0  [[o61 o1765 o1434 o2013 o1631 o2017 
>>>> o1808  60 ]]  #int:2
>>>>      o1769 ConL  === o0  [[o347 ]]  #long:-8589932784