[Bug middle-end/50066] New: [4.7 Regression] Bad signed long to unsigned long long conversion

[Bug middle-end/50066] New: [4.7 Regression] Bad signed long to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

             Bug #: 50066
           Summary: [4.7 Regression] Bad signed long to unsigned long long
                    conversion
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: hjl.tools@gmail.com


On Linux/ia32, I got

[hjl@gnu-6 gmp-1]$ cat x.c
#include <stdio.h>
#include <stdlib.h>
#include <limits.h>

extern unsigned long long foo (long);

int
main ()
{
  unsigned long long val = foo (LONG_MIN);
  printf ("0x%llx\n", val);
  if (val != 0x80000000)
    abort ();
  return 0;
}
[hjl@gnu-6 gmp-1]$ cat foo.c
unsigned long long
foo (signed long int val)
{
  return (unsigned long long) (unsigned long int) (val >= 0 ? val : -val);
}
[hjl@gnu-6 gmp-1]$ make 
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -m32 -O2   -c -o foo.o foo.c
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -m32 -O2   -c -o x.o x.c
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/ -m32 -O2 -o x foo.o x.o
./x
0xffffffff80000000
make: *** [all] Aborted (core dumped)
[hjl@gnu-6 gmp-1]$

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org
            Summary|[4.7 Regression] Bad signed |[4.7 Regression] Bad signed
                   |long to unsigned long long  |int to unsigned long long
                   |conversion                  |conversion

--- Comment #1 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-12 23:45:55 UTC ---
It may be caused by revision 176154:

http://gcc.gnu.org/ml/gcc-cvs/2011-07/msg00419.html

When replacing long with int, LONG_MIN with INT_MIN,
the testcase also fails on Linux/x86-64.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-08-13 04:32:16 UTC ---
- LONG_MIN is undefined.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-08-13 08:53:47 UTC ---
Indeed. Seems invalid to me.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 12:09:16 UTC ---
This code comes from mpz/set_si.c in gmp:

void
mpz_set_si (mpz_ptr dest, signed long int val)
{
  mp_size_t size;
  mp_limb_t vl;

  vl = (mp_limb_t) (unsigned long int) (val >= 0 ? val : -val);

mp_limb_t is unsigned 64bit and long is 32bit. It works
with all released GCC versions.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 14:27:58 UTC ---
Does this patch

---
diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index df7a9a2..f5e0a30 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -2065,6 +2065,12 @@ vrp_int_const_binop (enum tree_code code, tree val1,
tree
 val2)
       && is_overflow_infinity (val2))
     return NULL_TREE;

+      /* Punt integer subtraction with overflow on -MIN.  */ 
+      if (code == MINUS_EXPR
+      && INTEGRAL_TYPE_P (TREE_TYPE (res))
+      && (sgn1 < 0 || sgn2 < 0))
+    return NULL_TREE;
+
       /* Don't try to handle division or shifting of infinities.  */
       if ((code == TRUNC_DIV_EXPR
        || code == FLOOR_DIV_EXPR
---

make any senses?

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #6 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-08-13 15:28:22 UTC ---
On Sat, 13 Aug 2011, hjl.tools at gmail dot com wrote:

> --- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 12:09:16 UTC ---
> This code comes from mpz/set_si.c in gmp:
> 
> void
> mpz_set_si (mpz_ptr dest, signed long int val)
> {
>   mp_size_t size;
>   mp_limb_t vl;
> 
>   vl = (mp_limb_t) (unsigned long int) (val >= 0 ? val : -val);

So that GMP code is buggy.  Change it to

vl = (mp_limb_t) (val >= 0 ? (unsigned long int) val : -(unsigned long int)
val);

and it will be valid.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[joseph at codesourcery dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #7 from joseph at codesourcery dot com <joseph at codesourcery dot com> 2011-08-13 15:31:05 UTC ---
(The original code is of course valid if you use -fwrapv, so hopefully the 
problem optimization does not occur in that case.)

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #8 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 15:56:22 UTC ---
make_overflow_infinity sets to TYPE_MAX_VALUE/TYPE_MIN_VALUE.  Shouldn't
it set to TYPE_MAX_VALUE + 1/TYPE_MIN_VALUE - 1?

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #9 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 16:33:19 UTC ---
Shouldn't we check TREE_OVERFLOW:

diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c
index df7a9a2..4ec7e5b 100644
--- a/gcc/tree-vrp.c
+++ b/gcc/tree-vrp.c
@@ -7263,6 +7263,8 @@ simplify_conversion_using_ranges (gimple stmt)
   /* Get the value-range of the inner operand.  */
   innervr = get_value_range (innerop);
   if (innervr->type != VR_RANGE
+      || TREE_OVERFLOW (innervr->min)
+      || TREE_OVERFLOW (innervr->max)
       || TREE_CODE (innervr->min) != INTEGER_CST
       || TREE_CODE (innervr->max) != INTEGER_CST)
     return false;

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #10 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-08-13 17:51:33 UTC ---
HJL, the code as written in comment #0 is undefined.  Does the rewrite in
comment #6 work?  Also does adding -fwrapv work too?

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 18:11:56 UTC ---
Created attachment 25006
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25006
A patch

GMP code may be buggy.  But it works with all other compilers,
including GCC 4.6.0 and older. Is there any particular good
reason to ignore range overflow?  This patch checks range
overflow and caused no regressions on Linux/x86.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-08-13 18:17:24 UTC ---
(In reply to comment #11)
> Created attachment 25006 [details]
> A patch
> 
> GMP code may be buggy.  But it works with all other compilers,
> including GCC 4.6.0 and older. Is there any particular good
> reason to ignore range overflow?  This patch checks range
> overflow and caused no regressions on Linux/x86.

No Again we decided long ago to have overflow declared as being undefined and
ignoring range overflow is not what we decided.  It might work with other
compilers does not mean it is valid and well defined code.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #13 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 18:23:14 UTC ---
We should consider our users. GMP has been working with
GCC for a long time. Now it fails with GCC 4.7. It is
a very bad GCC 4.7 experience for user.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #14 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 18:32:53 UTC ---
What possible optimization do we gain by not checking
range overflow? Does anyone have a testcase to show it?

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

--- Comment #15 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-08-13 19:00:48 UTC ---
(In reply to comment #13)
> We should consider our users. GMP has been working with
> GCC for a long time. Now it fails with GCC 4.7. It is
> a very bad GCC 4.7 experience for user.

Yes and other packages failed when the original VPR and overflow was
introduced.  Why should this case be any different than all the others?

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID
   Target Milestone|---                         |4.7.0

--- Comment #16 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-14 09:49:01 UTC ---
Invalid.

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
   Last reconfirmed|                            |2011-11-30
         Resolution|INVALID                     |
     Ever Confirmed|0                           |1

--- Comment #17 from H.J. Lu <hjl.tools at gmail dot com> 2011-11-30 20:52:59 UTC ---
(In reply to comment #6)
> On Sat, 13 Aug 2011, hjl.tools at gmail dot com wrote:
> 
> > --- Comment #4 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-13 12:09:16 UTC ---
> > This code comes from mpz/set_si.c in gmp:
> > 
> > void
> > mpz_set_si (mpz_ptr dest, signed long int val)
> > {
> >   mp_size_t size;
> >   mp_limb_t vl;
> > 
> >   vl = (mp_limb_t) (unsigned long int) (val >= 0 ? val : -val);
> 
> So that GMP code is buggy.  Change it to
> 
> vl = (mp_limb_t) (val >= 0 ? (unsigned long int) val : -(unsigned long int)
> val);
> 
> and it will be valid.

It doesn't work:

[hjl@gnu-6 tmp]$ cat x.c
#include <stdio.h>
#include <limits.h>

unsigned long long
__attribute__ ((noinline))
foo (signed int v_digit)
{
  unsigned long long x;

  x = (unsigned long long) (v_digit >= 0 ? (unsigned int) v_digit : (unsigned
int) -v_digit);
  return x;
}

int
main ()
{
  unsigned long long x;

  x = foo (INT_MIN);
  printf ("%lld\n", x);
  x = foo (INT_MAX);
  printf ("%lld\n", x);
  return 0;
}
[hjl@gnu-6 tmp]$ /export/build/gnu/gcc-x32/release/usr/gcc-4.7.0-x32/bin/gcc
-O2 x.c
[hjl@gnu-6 tmp]$ ./a.out 
-2147483648
2147483647
[hjl@gnu-6 tmp]$ /export/build/gnu/gcc-x32/release/usr/gcc-4.7.0-x32/bin/gcc -O
x.c
[hjl@gnu-6 tmp]$ ./a.out 
2147483648
2147483647
[hjl@gnu-6 tmp]$

[Bug middle-end/50066] [4.7 Regression] Bad signed int to unsigned long long conversion

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50066

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|                            |INVALID

--- Comment #18 from Andrew Pinski <pinskia at gcc dot gnu.org> 2011-11-30 21:32:21 UTC ---
> (unsigned int) -v_digit

That is wrong it should be:
-(unsigned int) v_digit