This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
PATCH combine.c bug: Bad to transform (cmpop (minus A B) 0) into (cmpop A B).
- To: gcc-patches at gcc dot gnu dot org
- Subject: PATCH combine.c bug: Bad to transform (cmpop (minus A B) 0) into (cmpop A B).
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- Date: Mon, 3 Apr 2000 18:22:05 +0200
As the comment below the removed code in the patch says, there
are pathological cases involving integer (signed-)overflow.
It seems to have been overlooked that the pathological cases
involve comparing with zero. The testcase shows one, in which
bad code was generated, for ia32 and the soon-to-be-submitted
CRIS port; probably many more targets.
To wit, with unsigned long a = 0x7ffff001, b = 0x7ffff001, c = 0x1000;
(long) ((a + c) - b) > 0 was transformed to (a + c) > b
where ">" is a GT (signed compare), which looks right in
theory with infinite precision, but loses due to overflow.
The patch removes the bad special-case for zero in
simplify_comparison [MINUS]. Valid equality comparisons are
still transformed in the code after the patch, as seen in the
comment, with C = 0. Correct code is generated by the 2.95 branch.
No new regressions bootstrapping and checking i686-pc-linux-gnu.
Ok to install patch and testcase?
gcc.c-torture:
Mon Apr 3 16:53:52 2000 Hans-Peter Nilsson <hp@axis.com>
* execute/20000403-1.c: New test.
*** /dev/null Tue May 12 22:01:41 1998
--- 20000403-1.c Mon Apr 3 17:17:39 2000
***************
*** 0 ****
--- 1,29 ----
+ extern unsigned long aa[], bb[];
+
+ int seqgt (unsigned long a, unsigned short win, unsigned long b);
+
+ int seqgt2 (unsigned long a, unsigned short win, unsigned long b);
+
+ main()
+ {
+ if (! seqgt (*aa, 0x1000, *bb) || ! seqgt2 (*aa, 0x1000, *bb))
+ abort ();
+
+ exit (0);
+ }
+
+ int
+ seqgt (unsigned long a, unsigned short win, unsigned long b)
+ {
+ return (long) ((a + win) - b) > 0;
+ }
+
+ int
+ seqgt2 (unsigned long a, unsigned short win, unsigned long b)
+ {
+ long l = ((a + win) - b);
+ return l > 0;
+ }
+
+ unsigned long aa[] = { (1UL << (sizeof (long) * 8 - 1)) - 0xfff };
+ unsigned long bb[] = { (1UL << (sizeof (long) * 8 - 1)) - 0xfff };
gcc:
Mon Apr 3 16:34:54 2000 Hans-Peter Nilsson <hp@axis.com>
* combine.c (simplify_comparison) [MINUS]: Do not replace
all (op (minus A B) 0) with (op A B).
Index: combine.c
===================================================================
RCS file: /cvs/gcc/egcs/gcc/combine.c,v
retrieving revision 1.120
diff -c -p -c -p -4 -r1.120 combine.c
*** combine.c 2000/03/29 13:10:40 1.120
--- combine.c 2000/04/03 14:39:17
*************** simplify_comparison (code, pop0, pop1)
*** 10451,10466 ****
}
break;
case MINUS:
- /* (op (minus A B) 0) -> (op A B) */
- if (op1 == const0_rtx)
- {
- op1 = XEXP (op0, 1);
- op0 = XEXP (op0, 0);
- continue;
- }
-
/* (eq (minus A B) C) -> (eq A (plus B C)) or
(eq B (minus A C)), whichever simplifies. We can only do
this for equality comparisons due to pathological cases involving
overflows. */
--- 10451,10458 ----
brgds, H-P