[16] RFR(S): 8248552: C2 crashes with SIGFPE due to division by zero

[Christian Hagedorn]

Thank you Vladimir and Tobias for reviewing it again!

I filed an RFE [1] to investigate the change in PhiNode::Value() further.

Best regards,
Christian

[1] https://bugs.openjdk.java.net/browse/JDK-8250607

On 27.07.20 09:35, Tobias Hartmann wrote:
> +1
> 
> Best regards,
> Tobias
> 
> On 24.07.20 23:41, Vladimir Kozlov wrote:
>> Good.
>>
>> Thanks,
>> Vladimir K
>>
>> On 7/24/20 4:44 AM, Christian Hagedorn wrote:
>>> Hi Tobias
>>>
>>> Thank you for your review!
>>>
>>>> Please make sure to run performance testing.
>>>
>>> There is a repeated regression in the micros open crypto benchmark
>>> openjdk.bench.javax.crypto.small.SecureRandomBench.nextBytes with these two settings:
>>> - algorithm=SHA1PRNG-dataSize:64-provider:-shared:false
>>> - algorithm=SHA1PRNG-dataSize:64-provider:-shared:true
>>>
>>> Repeated runs with these two settings resulted in a regression between 1 and 2%. I could trace it
>>> back to the additional type filtering in PhiNode::Value() (webrev.02). This is only required for
>>> the assertion code and not for the bailout fix itself. When running performance testing with
>>> webrev.01, the regressions disappear.
>>>
>>> I therefore suggest to go with webrev.01 (without assertion code and type filtering) and file a
>>> new RFE to investigate the usage of type filtering in PhiNode::Value() for iv phis and why we get
>>> a performance regression in these two benchmark settings. In theory, I think it should be
>>> beneficial to narrow the type range of iv phis.
>>>
>>>> cfgnode.cpp:1083
>>>> - There's an extra whitespace before ","
>>>>
>>>> loopopts.cpp:84/86
>>>> - No need for extra brackets
>>>
>>> These are not present anymore in webrev.01.
>>>
>>> http://cr.openjdk.java.net/~chagedorn/8248552/webrev.01/
>>>
>>> Best regards,
>>> Christian
>>>
>>>
>>> On 20.07.20 11:14, Tobias Hartmann wrote:
>>>> Hi Christian,
>>>>
>>>> On 15.07.20 15:08, Christian Hagedorn wrote:
>>>>> http://cr.openjdk.java.net/~chagedorn/8248552/webrev.02/
>>>>
>>>> Looks good to me.
>>>>
>>>> Some code style comments:
>>>
>>>
>>>> Best regards,
>>>> Tobias
>>>>