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

[Christian Hagedorn]

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
>