diff mbox

Fix shorten_compare (PR c/48197)

Message ID 20110320124934.GP30899@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek March 20, 2011, 12:49 p.m. UTC
Hi!

As this testcase shows, shorten_compare misbehaves if *restype_ptr
is wider than one of the operands and get_narrower returns something even
narrower than opN.  If primopN is sign-extended to opN, but opN is unsigned,
unsignedpN is zero, but the mixed extension behaves differently from
sign-extension only.
This patch fixes it by ignoring what get_narrower did in that case.
For first zero-extending to opN and then sign-extending this isn't a
problem, as zero-extension means the sign-extension will fill bits with zero
anyway.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.6.1?

2011-03-20  Jakub Jelinek  <jakub@redhat.com>

	PR c/48197
	* c-common.c (shorten_compare): If primopN is first sign-extended
	to opN and then zero-extended to result type, set primopN to opN.

	* gcc.c-torture/execute/pr48197.c: New test.


	Jakub

Comments

Joseph Myers March 21, 2011, 4:31 p.m. UTC | #1
On Sun, 20 Mar 2011, Jakub Jelinek wrote:

> 2011-03-20  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/48197
> 	* c-common.c (shorten_compare): If primopN is first sign-extended
> 	to opN and then zero-extended to result type, set primopN to opN.
> 
> 	* gcc.c-torture/execute/pr48197.c: New test.

This is OK, in that I believe it is (a) correct and (b) a sensible place 
in the current context to fix this to avoid problems with shorten_compare 
not working with mixed extensions.  It's worth checking if this fixes PR 
42544 as well, since that also looks like it involves mixed extensions.

shorten_compare is of course a mess and could do with being moved to the 
middle-end (ideally gimple-fold, I think) - and with having a proper 
conceptual model of combinations of integer conversions rather than 
various ad hoc cases.  (The same applies to other shortening in front ends 
- and to anything using get_narrower at all.)  Suppose you separate out 
the floating-point bits so you are only dealing with sequences of integer 
conversions.  Then any such sequence can be reduced to the following (or a 
subset thereof): truncate to A bits, sign-extend to B bits and zero-extend 
to C bits (convert to a signed A-bit type, an unsigned B-bit type and an 
unsigned C-bit type).  I think these optimizations would be both (a) safer 
and (b) more likely to cover all cases if they worked by computing the 
values of A, B and C (and the associated inner operands that get truncated 
/ extended), using any available range information to determine that 
sign-extension is actually zero-extension in some cases, and then reasoned 
about what the best code to generate is in terms of those values.  (You 
can also integrate left and right shifts by constant values and bitwise 
AND, OR and XOR with constants into a more general version of this model - 
truncate, left shift, sign extend, XOR with a constant, force some bits to 
0 and some bits to 1 - though I don't know how useful that would be.)
diff mbox

Patch

--- gcc/c-family/c-common.c.jj	2011-03-16 18:29:52.000000000 +0100
+++ gcc/c-family/c-common.c	2011-03-19 15:52:10.000000000 +0100
@@ -3303,6 +3303,20 @@  shorten_compare (tree *op0_ptr, tree *op
   primop0 = get_narrower (op0, &unsignedp0);
   primop1 = get_narrower (op1, &unsignedp1);
 
+  /* If primopN is first sign-extended from primopN's precision to opN's
+     precision, then zero-extended from opN's precision to
+     *restype_ptr precision, shortenings might be invalid.  */
+  if (TYPE_PRECISION (TREE_TYPE (primop0)) < TYPE_PRECISION (TREE_TYPE (op0))
+      && TYPE_PRECISION (TREE_TYPE (op0)) < TYPE_PRECISION (*restype_ptr)
+      && !unsignedp0
+      && TYPE_UNSIGNED (TREE_TYPE (op0)))
+    primop0 = op0;
+  if (TYPE_PRECISION (TREE_TYPE (primop1)) < TYPE_PRECISION (TREE_TYPE (op1))
+      && TYPE_PRECISION (TREE_TYPE (op1)) < TYPE_PRECISION (*restype_ptr)
+      && !unsignedp1
+      && TYPE_UNSIGNED (TREE_TYPE (op1)))
+    primop1 = op1;
+
   /* Handle the case that OP0 does not *contain* a conversion
      but it *requires* conversion to FINAL_TYPE.  */
 
--- gcc/testsuite/gcc.c-torture/execute/pr48197.c.jj	2011-03-19 14:11:53.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr48197.c	2011-03-19 14:11:05.000000000 +0100
@@ -0,0 +1,25 @@ 
+/* PR c/48197 */
+
+extern void abort (void);
+static int y = 0x8000;
+
+int
+main ()
+{
+  unsigned int x = (short)y;
+  if (sizeof (0LL) == sizeof (0U))
+    return 0;
+  if (0LL > (0U ^ (short)-0x8000))
+    abort ();
+  if (0LL > (0U ^ x))
+    abort ();
+  if (0LL > (0U ^ (short)y))
+    abort ();
+  if ((0U ^ (short)-0x8000) < 0LL)
+    abort ();
+  if ((0U ^ x) < 0LL)
+    abort ();
+  if ((0U ^ (short)y) < 0LL)
+    abort ();
+  return 0;
+}