* [PATCH] Fix shorten_compare (PR c/48197)
@ 2011-03-20 12:49 Jakub Jelinek
2011-03-21 16:31 ` Joseph S. Myers
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2011-03-20 12:49 UTC (permalink / raw)
To: Joseph S. Myers, Jason Merrill; +Cc: gcc-patches
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.
--- 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;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix shorten_compare (PR c/48197)
2011-03-20 12:49 [PATCH] Fix shorten_compare (PR c/48197) Jakub Jelinek
@ 2011-03-21 16:31 ` Joseph S. Myers
2011-03-21 17:59 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Joseph S. Myers @ 2011-03-21 16:31 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Jason Merrill, gcc-patches
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.)
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix shorten_compare (PR c/48197)
2011-03-21 16:31 ` Joseph S. Myers
@ 2011-03-21 17:59 ` Jakub Jelinek
0 siblings, 0 replies; 3+ messages in thread
From: Jakub Jelinek @ 2011-03-21 17:59 UTC (permalink / raw)
To: Joseph S. Myers; +Cc: Jason Merrill, gcc-patches
On Mon, Mar 21, 2011 at 04:31:31PM +0000, Joseph S. Myers wrote:
> 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.
It does, so here is what I've actually committed:
2011-03-20 Jakub Jelinek <jakub@redhat.com>
PR c/42544
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/pr42544.c: New test.
* gcc.c-torture/execute/pr48197.c: New test.
--- gcc/c-family/c-common.c.jj 2011-03-21 13:00:03.000000000 +0100
+++ gcc/c-family/c-common.c 2011-03-21 18:29:23.000000000 +0100
@@ -3301,6 +3301,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/pr42544.c.jj 2011-03-21 18:25:01.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr42544.c 2011-03-21 18:24:29.000000000 +0100
@@ -0,0 +1,14 @@
+/* PR c/42544 */
+
+extern void abort (void);
+
+int
+main ()
+{
+ signed short s = -1;
+ if (sizeof (long long) == sizeof (unsigned int))
+ return 0;
+ if ((unsigned int) s >= 0x100000000ULL)
+ abort ();
+ return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr48197.c.jj 2011-03-21 18:29:23.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr48197.c 2011-03-21 18:29:23.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;
+}
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-03-21 17:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-03-20 12:49 [PATCH] Fix shorten_compare (PR c/48197) Jakub Jelinek
2011-03-21 16:31 ` Joseph S. Myers
2011-03-21 17:59 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).