PR66311: Fix extension in mpz->wide_int conversions

wi::from_mpz reads the absolute value of the mpz and then negates the
result if the mpz is negative.  When the top bit of the most-significant
HOST_WIDE_INT in the absolute value is set, we implicitly sign-
rather than zero-extend it to full precision.  For example,
1 << 63 gets mangled to (1 << prec) - (1 << 63).

This patch fixes that by ensuring we zero-extend instead.  The testcase
is taken from comment 15 in bugzilla (thanks FX).

Tested on x86_64-linux-gnu.  OK to install?

Thanks,
Richard

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.

On Wed, Aug 5, 2015 at 4:14 PM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> wi::from_mpz reads the absolute value of the mpz and then negates the
> result if the mpz is negative.  When the top bit of the most-significant
> HOST_WIDE_INT in the absolute value is set, we implicitly sign-
> rather than zero-extend it to full precision.  For example,
> 1 << 63 gets mangled to (1 << prec) - (1 << 63).
>
> This patch fixes that by ensuring we zero-extend instead.  The testcase
> is taken from comment 15 in bugzilla (thanks FX).
>
> Tested on x86_64-linux-gnu.  OK to install?

Ok.

Thanks,
Richard.

> Thanks,
> Richard
>
> 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.
>
> diff --git a/gcc/testsuite/gfortran.dg/pr66311.f90 b/gcc/testsuite/gfortran.dg/pr66311.f90
> new file mode 100644
> index 0000000..dc40cb6
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/pr66311.f90
> @@ -0,0 +1,60 @@
> +! { dg-do run }
> +! { dg-additional-options "-fno-range-check -w" }
> +!
> +! Check that we can print large constants
> +!
> +! "-fno-range-check -w" is used so the testcase compiles even with targets
> +! that don't support large integer kinds.
> +
> +program test
> +  use iso_fortran_env, only : ikinds => integer_kinds
> +  implicit none
> +
> +  ! Largest integer kind
> +  integer, parameter :: k = ikinds(size(ikinds))
> +  integer, parameter :: hk = k / 2
> +
> +  if (k <= 8) stop
> +
> +  call check(9000000000000000000_k, "9000000000000000000")
> +  call check(90000000000000000000_k, "90000000000000000000")
> +  call check(int(huge(1_hk), kind=k), "9223372036854775807")
> +  call check(2_k**63, "9223372036854775808")
> +  call check(10000000000000000000_k, "10000000000000000000")
> +  call check(18446744065119617024_k, "18446744065119617024")
> +  call check(2_k**64 - 1, "18446744073709551615")
> +  call check(2_k**64, "18446744073709551616")
> +  call check(20000000000000000000_k, "20000000000000000000")
> +  call check(huge(0_k), "170141183460469231731687303715884105727")
> +  call check(huge(0_k)-1, "170141183460469231731687303715884105726")
> +
> +  call check(-9000000000000000000_k, "-9000000000000000000")
> +  call check(-90000000000000000000_k, "-90000000000000000000")
> +  call check(-int(huge(1_hk), kind=k), "-9223372036854775807")
> +  call check(-2_k**63, "-9223372036854775808")
> +  call check(-10000000000000000000_k, "-10000000000000000000")
> +  call check(-18446744065119617024_k, "-18446744065119617024")
> +  call check(-(2_k**64 - 1), "-18446744073709551615")
> +  call check(-2_k**64, "-18446744073709551616")
> +  call check(-20000000000000000000_k, "-20000000000000000000")
> +  call check(-huge(0_k), "-170141183460469231731687303715884105727")
> +  call check(-(huge(0_k)-1), "-170141183460469231731687303715884105726")
> +  call check(-huge(0_k)-1, "-170141183460469231731687303715884105728")
> +
> +  call check(2_k * huge(1_hk), "18446744073709551614")
> +  call check((-2_k) * huge(1_hk), "-18446744073709551614")
> +
> +contains
> +
> +  subroutine check (i, str)
> +    implicit none
> +    integer(kind=k), intent(in), value :: i
> +    character(len=*), intent(in) :: str
> +
> +    character(len=100) :: buffer
> +    write(buffer,*) i
> +    if (adjustl(buffer) /= adjustl(str)) call abort
> +  end subroutine
> +
> +end
> +
> diff --git a/gcc/wide-int.cc b/gcc/wide-int.cc
> index 13ba10c..9a93660 100644
> --- a/gcc/wide-int.cc
> +++ b/gcc/wide-int.cc
> @@ -252,13 +252,15 @@ wi::from_mpz (const_tree type, mpz_t x, bool wrap)
>      }
>
>    /* Determine the number of unsigned HOST_WIDE_INTs that are required
> -     for representing the value.  The code to calculate count is
> +     for representing the absolute value.  The code to calculate count is
>       extracted from the GMP manual, section "Integer Import and Export":
>       http://gmplib.org/manual/Integer-Import-and-Export.html  */
>    numb = CHAR_BIT * sizeof (HOST_WIDE_INT);
>    count = (mpz_sizeinbase (x, 2) + numb - 1) / numb;
>    HOST_WIDE_INT *val = res.write_val ();
> -  /* Write directly to the wide_int storage if possible, otherwise leave
> +  /* Read the absolute value.
> +
> +     Write directly to the wide_int storage if possible, otherwise leave
>       GMP to allocate the memory for us.  It might be slightly more efficient
>       to use mpz_tdiv_r_2exp for the latter case, but the situation is
>       pathological and it seems safer to operate on the original mpz value
> @@ -276,7 +278,12 @@ wi::from_mpz (const_tree type, mpz_t x, bool wrap)
>        memcpy (val, valres, count * sizeof (HOST_WIDE_INT));
>        free (valres);
>      }
> -  res.set_len (canonize (val, count, prec));
> +  /* Zero-extend the absolute value to PREC bits.  */
> +  if (count < BLOCKS_NEEDED (prec) && val[count - 1] < 0)
> +    val[count++] = 0;
> +  else
> +    count = canonize (val, count, prec);
> +  res.set_len (count);
>
>    if (mpz_sgn (x) < 0)
>      res = -res;
>

On Aug 5, 2015, at 7:14 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> wi::from_mpz reads the absolute value of the mpz and then negates the
> result if the mpz is negative.  When the top bit of the most-significant
> HOST_WIDE_INT in the absolute value is set, we implicitly sign-
> rather than zero-extend it to full precision.  For example,
> 1 << 63 gets mangled to (1 << prec) - (1 << 63).
> 
> This patch fixes that by ensuring we zero-extend instead.  The testcase
> is taken from comment 15 in bugzilla (thanks FX).
> 
> Tested on x86_64-linux-gnu.  OK to install?

I’ve installed this on the gcc-5.x branch after regression testing on x86_64 linux.