[15] RFR(S): 8237950: C2 compilation fails with "Live Node limit exceeded limit" during ConvI2L::Ideal optimization

[Nils Eliasson]

+1

Best regards,
Nils

On 2020-06-18 17:43, Vladimir Kozlov wrote:
> Good.
>
> Thanks,
> Vladimir
>
> On 6/17/20 11:17 PM, Tobias Hartmann wrote:
>> Hi,
>>
>> performance testing revealed a slight regression with microbenchmarks 
>> due to ConvI2L nodes not
>> always being processed by IGVN and therefore not being optimized.
>>
>> Here's a new version that makes sure that such ConvI2L are always 
>> recorded for IGVN. Performance
>> numbers look good now.
>> http://cr.openjdk.java.net/~thartmann/8237950/webrev.01/
>>
>> Thanks,
>> Tobias
>>
>>
>> On 16.06.20 08:34, Tobias Hartmann wrote:
>>> Thanks Vladimir! I'll run our regular performance testing.
>>>
>>> Best regards,
>>> Tobias
>>>
>>> On 15.06.20 20:02, Vladimir Kozlov wrote:
>>>> +1
>>>>
>>>> I would suggest to do our regular performance testing to make sure 
>>>> there is no regression.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 6/15/20 6:31 AM, Nils Eliasson wrote:
>>>>> Hi Tobias,
>>>>>
>>>>> The change looks reasonable.
>>>>>
>>>>> Reviewed.
>>>>>
>>>>> Best regards,
>>>>> Nils
>>>>>
>>>>> On 2020-06-15 13:22, Tobias Hartmann wrote:
>>>>>> Hi,
>>>>>>
>>>>>> please review the following patch:
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8237950
>>>>>> http://cr.openjdk.java.net/~thartmann/8237950/webrev.00/
>>>>>>
>>>>>> A long chain of StringBuffer.append calls is optimized by C2's 
>>>>>> string concatenation optimization
>>>>>> which emits direct stores into the String internal byte array. 
>>>>>> GraphKit::array_element_address emits
>>>>>> ConvI2L nodes for the array index (see Compile::conv_I2X_index) 
>>>>>> without any range check dependent
>>>>>> CastII nodes because the bounds are known. As a result, the 
>>>>>> ConvI2L ideal optimization jumps in and
>>>>>> creates over 34000 new ConvI2L nodes while pushing them through 
>>>>>> the long chain of AddNodes. We hit
>>>>>> the node limit because during GVN, dead nodes are not removed.
>>>>>>
>>>>>> I propose to simply postpone that optimization to IGVN. This only 
>>>>>> affects array accesses emitted for
>>>>>> the string concat optimizations because "normal" array accesses 
>>>>>> have a range check dependent CastII
>>>>>> which blocks that ConvI2L optimization during parsing.
>>>>>>
>>>>>> Thanks,
>>>>>> Tobias
>>>>>