[Bug fortran/66311] New: [5 Regression] Problems with some integer(16) values

[Bug fortran/66311] New: [5 Regression] Problems with some integer(16) values

[gerhard.steinmetz.fortran@t-online.de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

            Bug ID: 66311
           Summary: [5 Regression] Problems with some integer(16) values
           Product: gcc
           Version: 5.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gerhard.steinmetz.fortran@t-online.de
  Target Milestone: ---

First of all, it's great to support integer(16).

But printing some numbers from range 2^63 to 2^64-1 with this snippet :

   program p
      print *, 9000000000000000000_16
      print *, huge(1_8)
      print *, 2_16**63
      print *, 10000000000000000000_16
      print *, 18446744065119617024_16
      print *, 2_16**64 - 1
      print *, 2_16**64
      print *, 20000000000000000000_16
   end


yields a wrong output for gfortran 5.1.1 (on SUSE Linux 13.2, 64 bit)

 9000000000000000000
  9223372036854775807
 -9223372036854775808
 -8446744073709551616
 -8589934592
 -1
 18446744073709551616
 20000000000000000000


Whereas with gfortran 4.9.0 and 4.8.3 the printed numbers are correct:

 9000000000000000000
  9223372036854775807
 9223372036854775808
 10000000000000000000
 18446744065119617024
 18446744073709551615
 18446744073709551616
 20000000000000000000


---

Same issue for the corresponding negative range :

   program p
      print *, -9000000000000000000_16
      print *, -huge(1_8)
      print *, -(2_16**63)
      print *, -10000000000000000000_16
      print *, -18446744065119617024_16
      print *, -(2_16**64 - 1)
      print *, -(2_16**64)
      print *, -20000000000000000000_16
   end


Result with gfortran 5.1.1 :

 -9000000000000000000
 -9223372036854775807
 9223372036854775808
 8446744073709551616
 8589934592
 1
 -18446744073709551616
 -20000000000000000000


Result with gfortran 4.9.0 and 4.8.3 :

 -9000000000000000000
 -9223372036854775807
 -9223372036854775808
 -10000000000000000000
 -18446744065119617024
 -18446744073709551615
 -18446744073709551616
 -20000000000000000000

[Bug fortran/66311] [5 Regression] Problems with some integer(16) values

[jvdelisle at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Jerry DeLisle <jvdelisle at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2015-05-28
                 CC|                            |jvdelisle at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #2 from Jerry DeLisle <jvdelisle at gcc dot gnu.org> ---
Confirmed.  Wow something broke here. I won't have tome until next week to
start a regression hunt.  Using -fdump-tree-original, one can see the different
resulting constants, so it is something in the front-end.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
   Target Milestone|---                         |5.2
            Summary|[5 Regression] Problems     |[5/6 Regression] Problems
                   |with some integer(16)       |with some integer(16)
                   |values                      |values

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[dominiq at lps dot ens.fr]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Dominique d'Humieres <dominiq at lps dot ens.fr> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mrs at gcc dot gnu.org,
                   |                            |rdsandiford at googlemail dot com,
                   |                            |zadeck at naturalbridge dot com

--- Comment #3 from Dominique d'Humieres <dominiq at lps dot ens.fr> ---
The regression occurred between revision r210073 (2014-05-05, OK) and r210124
(2014-05-06, wrong output). I suspect r210113: Merge in wide-int.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|5.2                         |5.3

--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 5.2 is being released, adjusting target milestone to 5.3.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[zadeck at naturalbridge dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #5 from Kenneth Zadeck <zadeck at naturalbridge dot com> ---
thanks



> On Jul 16, 2015, at 5:12 AM, rguenth at gcc dot gnu.org <gcc-bugzilla@gcc.gnu.org> wrote:
> 
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311
> 
> Richard Biener <rguenth at gcc dot gnu.org> changed:
> 
>           What    |Removed                     |Added
> ----------------------------------------------------------------------------
>   Target Milestone|5.2                         |5.3
> 
> --- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
> GCC 5.2 is being released, adjusting target milestone to 5.3.
> 
> -- 
> You are receiving this mail because:
> You are on the CC list for the bug.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Mikael Morin <mikael at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mikael at gcc dot gnu.org

--- Comment #7 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to Francois-Xavier Coudert from comment #6)
> I tracked the issue to the two calls to wi::from_mpz() and
> wide_int_to_tree() in gcc/fortran/trans-const.c:gfc_conv_mpz_to_tree().
> There the mpz value (which is correct) in translated into:
> 
>   wide_int_storage = {
>     val = ([0] = -2, [1] = 0, [2] = 56)
>     len = 1
>     precision = 128
>   }
> 
> which appears to be where the -2 comes from.

And in wi::from_mpz it probably comes from the mpz_sizeinbytes and mpz_export
functions not paying attention to the sign, as stated in the documentation:

https://gmplib.org/manual/Integer-Import-and-Export.html
> The sign of op is ignored, just the absolute value is exported. An application
> can use mpz_sgn to get the sign and handle it as desired. (see Integer
> Comparisons) 

https://gmplib.org/manual/Miscellaneous-Integer-Functions.html#Miscellaneous-Integer-Functions
> The sign of op is ignored, just the absolute value is used.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #8 from Mikael Morin <mikael at gcc dot gnu.org> ---
Created attachment 36121
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36121&action=edit
Beginning of a patch

The existing code mixes gmp allocation with wide_int allocation.
With the patch, an extra sign bit needs to be written in the most significant
word.
I don't know how to avoid the memory management nightmare.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[mrs at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #9 from mrs at gcc dot gnu.org <mrs at gcc dot gnu.org> ---
I've audited the patch for the memory management nightmares; we are safe with
it.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[zadeck at naturalbridge dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #10 from Kenneth Zadeck <zadeck at naturalbridge dot com> ---
I have audited the patch for the non memory management issues and it is 
approved.

thanks for doing this.

kenny

On 08/04/2015 07:38 AM, mikael at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311
>
> --- Comment #8 from Mikael Morin <mikael at gcc dot gnu.org> ---
> Created attachment 36121
>    --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36121&action=edit
> Beginning of a patch
>
> The existing code mixes gmp allocation with wide_int allocation.
> With the patch, an extra sign bit needs to be written in the most significant
> word.
> I don't know how to avoid the memory management nightmare.
>

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu.org

--- Comment #11 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Kenneth Zadeck from comment #10)
> I have audited the patch for the non memory management issues and it is 
> approved.

Please don't apply just yet, I'd like to take a closer look.
(Sorry for not doing it earlier.)

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[fxcoudert at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #13 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #12)
> Here's an alternative patch. I haven't yet tested it beyond
> an expanded version of the testcase, but I think it's easier
> to follow if we separate the "val != valres" and "need an extra
> zero block" logic.

I can confirm that the attached patch fixes all Fortran testcases reported in
this thread.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[mikael at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #14 from Mikael Morin <mikael at gcc dot gnu.org> ---
(In reply to rsandifo@gcc.gnu.org from comment #12)
> Created attachment 36128 [details]
> Alternative patch
> 
> Here's an alternative patch. I haven't yet tested it beyond
> an expanded version of the testcase, but I think it's easier
> to follow if we separate the "val != valres" and "need an extra
> zero block" logic.

Looks cleaner indeed. :-)
I'm still worried by the case where x is negative and fits in wide_int, but its
absolute value doesn't.
It's somehow the same as this case, except that the boundary is at the size of
wide_int (whatever it is) instead of 64 bits (the size of HOST_WIDE_INT) in
this case.
Not sure it matters.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[fxcoudert at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #15 from Francois-Xavier Coudert <fxcoudert at gcc dot gnu.org> ---
Created attachment 36129
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=36129&action=edit
Fortran testcase

Attached is a Fortran testcase, ready for inclusion in
gcc/testsuite/gfortran.dg
Please commit it along with the patch, when the issue is fixed.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #16 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
(In reply to Mikael Morin from comment #14)
> (In reply to rsandifo@gcc.gnu.org from comment #12)
> > Created attachment 36128 [details]
> > Alternative patch
> > 
> > Here's an alternative patch. I haven't yet tested it beyond
> > an expanded version of the testcase, but I think it's easier
> > to follow if we separate the "val != valres" and "need an extra
> > zero block" logic.
> 
> Looks cleaner indeed. :-)
> I'm still worried by the case where x is negative and fits in wide_int, but
> its absolute value doesn't.
> It's somehow the same as this case, except that the boundary is at the size
> of wide_int (whatever it is) instead of 64 bits (the size of HOST_WIDE_INT)
> in this case.
> Not sure it matters.

Well, in a sense there's no such wide_int, since wide_ints don't have
an inherent sign.  If a wide_int has precision N, the absolute value
of the minimum signed value is 1<<(N-1).  We'd read that in the same
way as we'd read an unsigned N-bit integer with value 1<<(N-1),
so the value would fit at that stage.  If the mpz is negative we then
negate the wide_int, which for the minimum signed value is a no-op.
We'd end up with an N-bit integer in which only bit N-1 is set.

The problem in the testcase is more to do with the decision that,
if an N-bit wide_int is represented with M<N bits, the other N-M
bits are implicitly sign extensions of bit M-1.  That's a pretty
arbitrary choice.  If we'd decided that the other bits were
implicitly zero then no patch would be needed.  (But of course
we'd then have had to write the other wide_int code differently,
and would have a less efficient representation of "negative"
(or large positive) numbers.)

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #17 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Author: rsandifo
Date: Wed Aug  5 14:23:42 2015
New Revision: 226632

URL: https://gcc.gnu.org/viewcvs?rev=226632&root=gcc&view=rev
Log:
gcc/
        PR middle-end/66311
        * wide-int.cc (wi::from_mpz): Make sure that absolute mpz value
        is zero- rather than sign-extended.

gcc/testsuite/
2015-08-05  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

        PR middle-end/66311
        * gfortran.dg/pr66311.f90: New file.

Added:
    trunk/gcc/testsuite/gfortran.dg/pr66311.f90
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/wide-int.cc

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rsandifo at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #18 from rsandifo at gcc dot gnu.org <rsandifo at gcc dot gnu.org> ---
Fixed on trunk.  I'll wait a few days to see whether there's any
unexpected fallout before backporting to gcc 5.

[Bug fortran/66311] [5/6 Regression] Problems with some integer(16) values

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4

[Bug middle-end/66311] [5 Regression] Problems with some integer(16) values

[mrs at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

--- Comment #20 from mrs at gcc dot gnu.org <mrs at gcc dot gnu.org> ---
Author: mrs
Date: Sat Oct 17 03:55:03 2015
New Revision: 228932

URL: https://gcc.gnu.org/viewcvs?rev=228932&root=gcc&view=rev
Log:
2015-10-16  Richard Sandiford  <richard.sandiford@arm.com>

        PR middle-end/66311
        * wide-int.cc (wi::from_mpz): Make sure that absolute mpz value
        is zero- rather than sign-extended.

testsuite:
2015-10-16  Francois-Xavier Coudert  <fxcoudert@gcc.gnu.org>

        PR middle-end/66311
        * gfortran.dg/pr66311.f90: New file.

Added:
    branches/gcc-5-branch/gcc/testsuite/gfortran.dg/pr66311.f90
Modified:
    branches/gcc-5-branch/gcc/ChangeLog
    branches/gcc-5-branch/gcc/testsuite/ChangeLog
    branches/gcc-5-branch/gcc/wide-int.cc

[Bug middle-end/66311] [5 Regression] Problems with some integer(16) values

[mrs at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

mrs at gcc dot gnu.org <mrs at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
      Known to work|                            |5.3.0
         Resolution|---                         |FIXED
      Known to fail|                            |5.2.0

--- Comment #21 from mrs at gcc dot gnu.org <mrs at gcc dot gnu.org> ---
Fixed.

[Bug middle-end/66311] [5 Regression] Problems with some integer(16) values

[clyon at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66311

Christophe Lyon <clyon at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clyon at gcc dot gnu.org

--- Comment #22 from Christophe Lyon <clyon at gcc dot gnu.org> ---
I see the testcase fail at execution time for arm* targets, and it passes in
aarch64* targets.

I'm using qemu in both cases, if that matters.