Of course, some computations are wrong, too.

For example :

   program p
      integer(8), parameter :: z = huge(1_8)
      print *, 2_16 * z
      print *, 2 * int(z, 16)
      print *, 2_16 * int(z, 16)
      print *, int(2, 16) * int(z, 16)
   end


Result with gfortran 5.1.1 :

 -2
 -2
 -2
 -2

Result with gfortran 4.9.0 and 4.8.3 :

 18446744073709551614
 18446744073709551614
 18446744073709551614
 18446744073709551614


---

Or this variation :

   program p
      integer(16), parameter :: z = huge(1_8)
      print *, 2_16 * z
      print *, 2 * int(z, 16)
      print *, 2_16 * int(z, 16)
      print *, int(2, 16) * int(z, 16)
   end

Result with gfortran 5.1.1 :

 -2
 -2
 -2
 -2

Result with gfortran 4.9.0 and 4.8.3 :

 18446744073709551614
 18446744073709551614
 18446744073709551614
 18446744073709551614

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.

The regression occurred between revision r210073 (2014-05-05, OK) and r210124 (2014-05-06, wrong output). I suspect r210113: Merge in wide-int.

GCC 5.2 is being released, adjusting target milestone to 5.3.

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.

Definitely a middle-end issue. We get it right on the front-end, the variable initialization gets screwed when translated later. Reduced testcase:

$ cat a.f90
program test
  integer(kind=16) :: z = 18446744073709551614_16
  print *, z
end
$ ./usr/local/bin/gfortran -fdump-parse-tree -fdump-tree-original a.f90 && ./a.out
 -2


The "parse tree" clearly shows that z has value of 18446744073709551614_16. But in the a.f90.003t.original, we get the wrong initialization:

  static integer(kind=16) z = -2;


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. Probably not seen outside Fortran because people don't use this conversion routine in C (because there is not way to input literal __int128 values in C).

Can someone familiar with wide-int look into it?

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

Created attachment 36121 [details]
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.

I've audited the patch for the memory management nightmares; we are safe with it.

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 [details]
>    --> 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.
>

(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.)

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.

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

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

Created attachment 36129 [details]
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.

(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.)

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

Fixed on trunk.  I'll wait a few days to see whether there's any
unexpected fallout before backporting to gcc 5.

Still waiting for backport.

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

Fixed.

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.