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

[Vladimir Kozlov]

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
>>