This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix bug in fold_div_compare


This is a regression from 3.x present on all active branches.

extern void abort(void);

typedef unsigned long long uint64;

int very_large_value (uint64 t)
{
  return (t / 1000000000ULL) > 9223372037ULL;
}

int main(void)
{
  uint64 t = 0xC000000000000000LL;

  if (!very_large_value (t))
    abort ();

  return 0;
}

eric@linux:~/build/gcc/native> gcc/xgcc -v
Using built-in specs.
Target: x86_64-suse-linux
Configured with: /home/eric/svn/gcc/configure x86_64-suse-linux 
--prefix=/home/eric/install/gcc 
--enable-languages=c,c++,objc,obj-c++,java,fortran,ada --enable-__cxa_atexit 
--enable-checking=assert,gc,misc,rtl,rtlflag,runtime,tree
Thread model: posix
gcc version 4.2.0 20061010 (experimental)
eric@linux:~/build/gcc/native> gcc/xgcc -Bgcc -o t t.c
eric@linux:~/build/gcc/native> ./t
eric@linux:~/build/gcc/native>

eric@linux:~/build/gcc/native32> gcc/xgcc -v
Using built-in specs.
Target: i586-suse-linux
Configured with: /home/eric/svn/gcc/configure i586-suse-linux 
--prefix=/home/eric/install/gcc --with-as=/usr/i586-suse-linux/bin/as 
--with-ld=/usr/i586-suse-linux/bin/ld --enable-__cxa_atexit 
--disable-libmudflap 
--enable-checking=assert,gc,misc,rtl,rtlflag,runtime,tree 
--enable-languages=c --disable-bootstrap
Thread model: posix
gcc version 4.2.0 20061010 (experimental)
eric@linux:~/build/gcc/native32> gcc/xgcc -Bgcc -o t t.c
eric@linux:~/build/gcc/native32> ./t
Aborted

So there is a problem on 32-bit hosts in fold_div_compare.  Excerpt:

  /* We have to do this the hard way to detect unsigned overflow.
     prod = int_const_binop (MULT_EXPR, arg01, arg1, 0);  */
  overflow = mul_double (TREE_INT_CST_LOW (arg01),
			 TREE_INT_CST_HIGH (arg01),
			 TREE_INT_CST_LOW (arg1),
			 TREE_INT_CST_HIGH (arg1), &lpart, &hpart);

But on other hand:

/* Multiply two doubleword integers with doubleword result.
   Return nonzero if the operation overflows, assuming it's signed.
   Each argument is given as two `HOST_WIDE_INT' pieces.
   One argument is L1 and H1; the other, L2 and H2.
   The value is stored as two `HOST_WIDE_INT' pieces in *LV and *HV.  */

int
mul_double (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
	    unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
	    unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv)

We are using a function wired for signed types to detect unsigned overflow.
And of course 1000000000ULL * 9223372037ULL overflows in the signed 64-bit 
arithmetics but not in the unsigned one.

Hence the proposed fix, moderately tested on x86/x86_64-suse-linux.

OK for all active branches after a complete testing cycle?


2006-10-10  Eric Botcazou  <ebotcazou@adacore.com>

	* fold-const.c (add_double): Rename to add_double_with_sign.
	Add 'unsigned_p' parameter and take it into account for the overflow.
	(mul_double): Rename to mul_double_with_sign. 
	Add 'unsigned_p' parameter and take it into account for the overflow.
	(fold_div_compare): Call add_double_with_sign instead of add_double
	and mul_double_with_sign instead of mul_double, passing them the
	unsignedness of the type.
	* tree.h (add_double): Macroize.
	(add_double_with_sign): New prototype.
	(mul_double): Macroize.
	(mul_double_with_sign): New prototype.


2006-10-10  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc.dg/div-compare-1.c: New test.


-- 
Eric Botcazou
Index: fold-const.c
===================================================================
--- fold-const.c	(revision 117595)
+++ fold-const.c	(working copy)
@@ -291,14 +291,16 @@ force_fit_type (tree t, int overflowable
 }
 
 /* Add two doubleword integers with doubleword result.
+   Return nonzero if the operation overflows according to UNSIGNED_P.
    Each argument is given as two `HOST_WIDE_INT' pieces.
    One argument is L1 and H1; the other, L2 and H2.
    The value is stored as two `HOST_WIDE_INT' pieces in *LV and *HV.  */
 
 int
-add_double (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
-	    unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
-	    unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv)
+add_double_with_sign (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
+		      unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
+		      unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv,
+		      bool unsigned_p)
 {
   unsigned HOST_WIDE_INT l;
   HOST_WIDE_INT h;
@@ -308,7 +310,11 @@ add_double (unsigned HOST_WIDE_INT l1, H
 
   *lv = l;
   *hv = h;
-  return OVERFLOW_SUM_SIGN (h1, h2, h);
+
+  if (unsigned_p)
+    return (unsigned HOST_WIDE_INT) h < (unsigned HOST_WIDE_INT) h1;
+  else
+    return OVERFLOW_SUM_SIGN (h1, h2, h);
 }
 
 /* Negate a doubleword integer with doubleword result.
@@ -335,15 +341,16 @@ neg_double (unsigned HOST_WIDE_INT l1, H
 }
 
 /* Multiply two doubleword integers with doubleword result.
-   Return nonzero if the operation overflows, assuming it's signed.
+   Return nonzero if the operation overflows according to UNSIGNED_P.
    Each argument is given as two `HOST_WIDE_INT' pieces.
    One argument is L1 and H1; the other, L2 and H2.
    The value is stored as two `HOST_WIDE_INT' pieces in *LV and *HV.  */
 
 int
-mul_double (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
-	    unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
-	    unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv)
+mul_double_with_sign (unsigned HOST_WIDE_INT l1, HOST_WIDE_INT h1,
+		      unsigned HOST_WIDE_INT l2, HOST_WIDE_INT h2,
+		      unsigned HOST_WIDE_INT *lv, HOST_WIDE_INT *hv,
+		      bool unsigned_p)
 {
   HOST_WIDE_INT arg1[4];
   HOST_WIDE_INT arg2[4];
@@ -374,11 +381,15 @@ mul_double (unsigned HOST_WIDE_INT l1, H
       prod[i + 4] = carry;
     }
 
-  decode (prod, lv, hv);	/* This ignores prod[4] through prod[4*2-1] */
-
-  /* Check for overflow by calculating the top half of the answer in full;
-     it should agree with the low half's sign bit.  */
+  decode (prod, lv, hv);
   decode (prod + 4, &toplow, &tophigh);
+
+  /* Unsigned overflow is immediate.  */
+  if (unsigned_p)
+    return (toplow | tophigh) != 0;
+
+  /* Check for signed overflow by calculating the signed representation of the
+     top half of the result; it should agree with the low half's sign bit.  */
   if (h1 < 0)
     {
       neg_double (l2, h2, &neglow, &neghigh);
@@ -6083,30 +6094,32 @@ fold_div_compare (enum tree_code code, t
   tree arg01 = TREE_OPERAND (arg0, 1);
   unsigned HOST_WIDE_INT lpart;
   HOST_WIDE_INT hpart;
+  bool unsigned_p = TYPE_UNSIGNED (TREE_TYPE (arg0));
   bool neg_overflow;
   int overflow;
 
   /* We have to do this the hard way to detect unsigned overflow.
      prod = int_const_binop (MULT_EXPR, arg01, arg1, 0);  */
-  overflow = mul_double (TREE_INT_CST_LOW (arg01),
-			 TREE_INT_CST_HIGH (arg01),
-			 TREE_INT_CST_LOW (arg1),
-			 TREE_INT_CST_HIGH (arg1), &lpart, &hpart);
+  overflow = mul_double_with_sign (TREE_INT_CST_LOW (arg01),
+				   TREE_INT_CST_HIGH (arg01),
+				   TREE_INT_CST_LOW (arg1),
+				   TREE_INT_CST_HIGH (arg1),
+				   &lpart, &hpart, unsigned_p);
   prod = build_int_cst_wide (TREE_TYPE (arg00), lpart, hpart);
   prod = force_fit_type (prod, -1, overflow, false);
   neg_overflow = false;
 
-  if (TYPE_UNSIGNED (TREE_TYPE (arg0)))
+  if (unsigned_p)
     {
       tmp = int_const_binop (MINUS_EXPR, arg01, integer_one_node, 0);
       lo = prod;
 
       /* Likewise hi = int_const_binop (PLUS_EXPR, prod, tmp, 0).  */
-      overflow = add_double (TREE_INT_CST_LOW (prod),
-			     TREE_INT_CST_HIGH (prod),
-			     TREE_INT_CST_LOW (tmp),
-			     TREE_INT_CST_HIGH (tmp),
-			     &lpart, &hpart);
+      overflow = add_double_with_sign (TREE_INT_CST_LOW (prod),
+				       TREE_INT_CST_HIGH (prod),
+				       TREE_INT_CST_LOW (tmp),
+				       TREE_INT_CST_HIGH (tmp),
+				       &lpart, &hpart, unsigned_p);
       hi = build_int_cst_wide (TREE_TYPE (arg00), lpart, hpart);
       hi = force_fit_type (hi, -1, overflow | TREE_OVERFLOW (prod),
 			   TREE_CONSTANT_OVERFLOW (prod));
Index: tree.h
===================================================================
--- tree.h	(revision 117595)
+++ tree.h	(working copy)
@@ -4210,14 +4210,20 @@ extern tree fold_indirect_ref_1 (tree, t
 
 extern tree force_fit_type (tree, int, bool, bool);
 
-extern int add_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
-		       unsigned HOST_WIDE_INT, HOST_WIDE_INT,
-		       unsigned HOST_WIDE_INT *, HOST_WIDE_INT *);
+extern int add_double_with_sign (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
+				 unsigned HOST_WIDE_INT, HOST_WIDE_INT,
+				 unsigned HOST_WIDE_INT *, HOST_WIDE_INT *,
+				 bool);
+#define add_double(l1,h1,l2,h2,lv,hv) \
+  add_double_with_sign (l1, h1, l2, h2, lv, hv, false)
 extern int neg_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
 		       unsigned HOST_WIDE_INT *, HOST_WIDE_INT *);
-extern int mul_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
-		       unsigned HOST_WIDE_INT, HOST_WIDE_INT,
-		       unsigned HOST_WIDE_INT *, HOST_WIDE_INT *);
+extern int mul_double_with_sign (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
+				 unsigned HOST_WIDE_INT, HOST_WIDE_INT,
+				 unsigned HOST_WIDE_INT *, HOST_WIDE_INT *,
+				 bool);
+#define mul_double(l1,h1,l2,h2,lv,hv) \
+  mul_double_with_sign (l1, h1, l2, h2, lv, hv, false)
 extern void lshift_double (unsigned HOST_WIDE_INT, HOST_WIDE_INT,
 			   HOST_WIDE_INT, unsigned int,
 			   unsigned HOST_WIDE_INT *, HOST_WIDE_INT *, int);
extern void abort(void);

typedef unsigned long long uint64;

int very_large_value (uint64 t)
{
  return (t / 1000000000ULL) > 9223372037ULL;
}

int main(void)
{
  uint64 t = 0xC000000000000000LL;

  if (!very_large_value (t))
    abort ();

  return 0;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]