On Wed, Aug 5, 2015 at 4:14 PM, Richard Sandiford
<richard.sandif...@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 <fxcoud...@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;
>
