RFR(S): 8166742 : SIGFPE in C2 Loop IV elimination

[Vladimir Kozlov]

On 9/27/16 1:57 PM, Chuck Rasbold wrote:
> Sorry for not being transparent enough.  Here's an external reference
> that describes the problem
> that is being encountered by the division:
>
> https://www.gnu.org/software/autoconf/manual/autoconf-2.67/html_node/Signed-Integer-Division.html
>
> That's why the original fix targeted a very specific case.  One can't
> represent ratio_con as a 32 bit value in that case.
> Worse, trying to compute it by division causes a SIGFPE.
>
> Do you think the revised code below is as straightforward as the original?

Okay, looks like it is very special only one case and not a range of 
cases. Lets use your original fix then.

I will sponsor it.

Thanks,
Vladimir

>
> -- Chuck
>
> On Tue, Sep 27, 2016 at 9:48 AM, Vladimir Kozlov
> <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>> wrote:
>
>     So why it is SIGFPE when both values are 'int'?
>
>     I thought it is incorrect results cause SIGFPE that is why I
>     suggested to check for integer overflow.
>
>     Lets then go with your second suggested change here. But let check
>     that ratio is small first and do cast to (jint) otherwise the long
>     check is useless:
>
>       // The ratio of the two strides cannot be represented as an int
>       // if stride_con2 is min_int and stride_con is -1.
>       jlong ratio_conl = ((jlong)stride_con2 / stride_con);
>
>       if ((ratio_conl < 0x80000000L) &&
>           (jint)(ratio_conl * stride_con) == stride_con2) { // Check for
>     exact
>          jint ratio_con = (jint)ratio_conl;
>
>     Thanks,
>     Vladimir
>
>     On 9/27/16 7:56 AM, Chuck Rasbold wrote:
>
>
>
>         On Mon, Sep 26, 2016 at 5:35 PM, Vladimir Kozlov
>         <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>
>         <mailto:vladimir.kozlov at oracle.com
>         <mailto:vladimir.kozlov at oracle.com>>> wrote:
>
>             Slightly different (cast after /) and jlong type:
>
>               jlong ratio_conl = (jlong) (stride_con2 / stride_con);
>
>
>         The division above won't work (at least, it raises a SIGFPE on
>         my Linux
>         x86 platform) when stride_con2 == min_jint   and stride_con == -1.
>
>
>               if ((ratio_conl * stride_con) == (jlong)stride_con2) { //
>         Check
>             for exact
>
>
>         What would be the value of ratio_conl such that this test fails?  I
>         think I'm missing something...
>
>         -- Chuck
>
>
>             Vladimir
>
>             On 9/26/16 5:01 PM, Chuck Rasbold wrote:
>
>                 Just to confirm, are you suggesting that the ratio be first
>                 computed as a 64 bit quantity, effectively along the
>         lines of...
>
>                    long ratio_conl = ((long) stride_con2) / stride_con;
>
>                    if ((ratio_conl * stride_con) == stride_con2 &&
>                        ratio_conl < 0x8000000 ) { // Check for exact
>                      int ratio_con = (int) ratio_conl;
>
>
>                 On Mon, Sep 26, 2016 at 3:45 PM, Vladimir Kozlov
>                 <vladimir.kozlov at oracle.com
>         <mailto:vladimir.kozlov at oracle.com>
>         <mailto:vladimir.kozlov at oracle.com
>         <mailto:vladimir.kozlov at oracle.com>>
>                 <mailto:vladimir.kozlov at oracle.com
>         <mailto:vladimir.kozlov at oracle.com>
>                 <mailto:vladimir.kozlov at oracle.com
>         <mailto:vladimir.kozlov at oracle.com>>>> wrote:
>
>                     Hi Chuck
>
>                     Can you do 'long' arithmetic in existing condition
>         to catch
>                 integer overflow instead?
>
>                     if ((ratio_con * stride_con) == stride_con2) { //
>         Check for
>                 exact
>
>                     thanks,
>                     Vladimir
>
>
>
>                     On 9/26/16 3:18 PM, Chuck Rasbold wrote:
>
>                         A small fix for an edge case crash in C2...
>
>                         Bug:
>         https://bugs.openjdk.java.net/browse/JDK-8166742
>         <https://bugs.openjdk.java.net/browse/JDK-8166742>
>                 <https://bugs.openjdk.java.net/browse/JDK-8166742
>         <https://bugs.openjdk.java.net/browse/JDK-8166742>>
>                 <https://bugs.openjdk.java.net/browse/JDK-8166742
>         <https://bugs.openjdk.java.net/browse/JDK-8166742>
>                 <https://bugs.openjdk.java.net/browse/JDK-8166742
>         <https://bugs.openjdk.java.net/browse/JDK-8166742>>>
>                         Webrev:
>                 http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/
>         <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/>
>                 <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/
>         <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/>>
>                 <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/
>         <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/>
>                 <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/
>         <http://cr.openjdk.java.net/~rasbold/8166742/webrev.00/>>>
>
>                         Requesting a sponsor and reviews.  Thanks.
>
>                         -- Chuck
>
>
>
>