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 folding of ABS_EXPR


Hi!

The testcase below is miscompiled on various 64-bit arches at any
optimization level in GCC 3.[234].  The problem is still there in HEAD,
but doesn't show up on this testcase.

When folding a[abs (i - 10) - 11], extract_muldiv_1 is called
on abs (i - 10) (which has type int) with c == 4 and wide_type
unsigned long (as it is being used as index into the array).
extract_muldiv_1 optimizes this into abs ((unsigned long) i * 4 - 40),
where abs itself has unsigned long type too (tree.def mandates
that type of ABS_EXPR and its operands are the same).
But several places in the compiler when they see either that
ABS_EXPR's operand type or ABS_EXPR's own type are unsigned
just optimize the ABS_EXPR out, returning just its arguments.

The following patch prevents this, in this case by folding it into
(unsigned long) (abs ((long) i * 4 - 40)).
Alternatively if computing the operand * c in signed_type (ctype)
is deemed unsafe (note that we know that ctype's MODE_SIZE >
type's MODE_SIZE, so I hope it is safe), we could do what MIN_EXPR
and MAX_EXPR do, i.e. punt whenever ctype's sign is different from
type's.  But if it is safe, this patch results in one less instruction
than when punting, at least on x86-64.

Ok for 3.4/HEAD if testing succeeds?

2004-11-26  Jakub Jelinek  <jakub@redhat.com>

	* fold-const.c (extract_muldiv_1) <case ABS_EXPR>: If ctype is
	unsigned and type signed, build ABS_EXPR with signed_type (ctype)
	and only afterwards convert to ctype.

	* gcc.c-torture/execute/20041126-1.c: New test.

--- gcc/fold-const.c.jj	2004-11-26 16:45:23.793325147 +0100
+++ gcc/fold-const.c	2004-11-26 16:52:33.848774002 +0100
@@ -4590,7 +4590,21 @@ extract_muldiv_1 (tree t, tree c, enum t
 	return t1;
       break;
 
-    case NEGATE_EXPR:  case ABS_EXPR:
+    case ABS_EXPR:
+      /* If widening the type changes it from signed to unsigned, then we
+         must avoid building ABS_EXPR itself as unsigned.  */
+      if (TREE_UNSIGNED (ctype) && !TREE_UNSIGNED (type))
+        {
+          tree cstype = (*lang_hooks.types.signed_type) (ctype);
+          if ((t1 = extract_muldiv (op0, c, code, cstype)) != 0)
+            {
+              t1 = fold (build1 (tcode, cstype, fold_convert (cstype, t1)));
+              return fold_convert (ctype, t1);
+            }
+          break;
+        }
+      /* FALLTHROUGH */
+    case NEGATE_EXPR:
       if ((t1 = extract_muldiv (op0, c, code, wide_type)) != 0)
 	return fold (build1 (tcode, ctype, fold_convert (ctype, t1)));
       break;
--- gcc/testsuite/gcc.c-torture/execute/20041126-1.c.jj	2004-11-22 15:53:47.211447624 +0100
+++ gcc/testsuite/gcc.c-torture/execute/20041126-1.c	2004-11-26 15:32:33.056539522 +0100
@@ -0,0 +1,26 @@
+extern int abs (int);
+extern void abort (void);
+
+void
+check (int *p)
+{
+  int i;
+  for (i = 0; i < 5; ++i)
+    if (p[i])
+      abort ();
+  for (; i < 10; ++i)
+    if (p[i] != i + 1)
+      abort ();
+}
+
+int
+main (void)
+{
+  int a[10] = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };
+  int i;
+
+  for (i = -5; i < 0; i++)
+    a[abs (i - 10) - 11] = 0;
+  check (a);
+  return 0;
+}

	Jakub


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