[PATCH] Fix incorrect optab expansion for ulcmp on ARM.

Carlos O'Donell carlos@codesourcery.com
Fri Jan 27 17:35:00 GMT 2006


ARM has an eabi helper function for doing unsigned long comparisons
(__eabi_ulcmp) and according to the ABI this returns an unbiased result. We
trigger the use of this helper function by doing a division that
evalutes to a comparison.  The return of the helper function is compared
and a conditional move emitted. 

The problem is that the unbiased result requires a different set of
conditional moves, and this involves changing the comparison code all
the way up the list of callers. However, the callers expect that
compare_from_rtx produces an equivalent comparison. 

The simplest fix is to add one to the return thereby biasing the result.

The technically correct fix is to hoist this all the way up, and allow
the lower level routines to modify the comparison code and the rest of
the callers should notice this and make appropriate changes or not care.
This involves an audit of the callers. I consider this an optimziation
and a case for later study.

Ok for mainline?

Ok for 4.1?
-----------
ARM is the only architecture that has a helper function that returns
an unbiased result. This fix is trivial enough that we can show it
doesn't effect any of the other arches. Can we consider this a
regression fix since it used to work until the helper was added :}

Tested with no regressions on x86_64-pc-linux-gnu and arm-none-eabi.

Cheers,
Carlos.
-- 
Carlos O'Donell
CodeSourcery
carlos@codesourcery.com
(650) 331-3385 x716

gcc/

2006-01-27  Carlos O'Donell  <carlos@codesourcery.com>

	* optabs.c (prepare_cmp_insn): If unbaised and unsigned then bias
	the comparison routine return.
	
gcc/testsuite/

2006-01-27  Carlos O'Donell  <carlos@codesourcery.com>

	* gcc.dg/unsigned-long-compare.c: New test.

Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	(revision 110300)
+++ gcc/optabs.c	(working copy)
@@ -3711,18 +3711,24 @@
       result = emit_library_call_value (libfunc, NULL_RTX, LCT_CONST_MAKE_BLOCK,
 					word_mode, 2, x, mode, y, mode);
 
+      /* There are two kinds of comparison routines. Biased routines
+	 return 0/1/2, and unbiased routines return -1/0/1. Other parts
+	 of gcc expect that the comparison operation is equivalent
+	 to the modified comparison. For signed comparisons compare the 
+	 result against 1 in the unbiased case, and zero in the biased
+	 case. For unsigned comparisons always compare against 1 after
+	 biasing the unbased result by adding 1. This gives us a way to
+	 represent LTU. */
       *px = result;
       *pmode = word_mode;
-      if (TARGET_LIB_INT_CMP_BIASED)
-	/* Integer comparison returns a result that must be compared
-	   against 1, so that even if we do an unsigned compare
-	   afterward, there is still a value that can represent the
-	   result "less than".  */
-	*py = const1_rtx;
-      else
+      *py = const1_rtx;
+
+      if (!TARGET_LIB_INT_CMP_BIASED)
 	{
-	  *py = const0_rtx;
-	  *punsignedp = 1;
+	  if (*punsignedp)
+	    *px = plus_constant (result, 1);  
+	  else
+	    *py = const0_rtx;
 	}
       return;
     }
Index: gcc/testsuite/gcc.dg/unsigned-long-compare.c
===================================================================
--- gcc/testsuite/gcc.dg/unsigned-long-compare.c	(revision 0)
+++ gcc/testsuite/gcc.dg/unsigned-long-compare.c	(revision 0)
@@ -0,0 +1,24 @@
+/* Copyright (C) 2006 Free Software Foundation, Inc. */
+/* Contributed by Carlos O'Donell on 2006-01-27 */
+
+/* Test a division corner case where the expression simplifies
+   to a comparison, and the optab expansion is wrong. The optab 
+   expansion emits a function whose return is unbiased and needs
+   adjustment. */
+/* Origin: Carlos O'Donell <carlos@codesourcery.com> */
+/* { dg-do run { target arm-*-*eabi* } } */
+/* { dg-options "" } */
+#include <stdlib.h>
+
+#define BIG_CONSTANT 0xFFFFFFFF80000000ULL
+
+int main (void)
+{
+  unsigned long long OneULL = 1ULL;
+  unsigned long long result;
+
+  result = OneULL / BIG_CONSTANT; 
+  if (result)
+    abort ();
+  exit (0);
+}



More information about the Gcc-patches mailing list