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

[Vladimir Ivanov]

>      Since JDK 12 has the same issue, will this fix be integrated into this repo?

I don't plan to integrate it into jdk12.

My reasoning is:

   (1) it's a long-standing bug (from day 1 on x64?) with very low 
likelihood of exposure
         * was found only recently using fuzzers
         * no similar crashes reported before

   (2) JDK 12 is in RDP2 phase and is open only for P1–P2 bug fixes

Though the bug technically meets RDP2 criteria, I don't see it as a 
critical issue for the release in a late development phase.

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