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

[Chuck Rasbold]

Thanks, Vladimir!

On Wed, Sep 28, 2016 at 9:47 AM, Vladimir Kozlov <vladimir.kozlov at oracle.com
> wrote:

> 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/h
>> tml_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
>>
>>
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20160928/043aff4f/attachment.html>