public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: Michael Meissner <meissner@linux.vnet.ibm.com>
Cc: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix combiner issues with shifts (PR c/37924, take 2)
Date: Tue, 28 Oct 2008 12:38:00 -0000	[thread overview]
Message-ID: <20081028112718.GZ14706@tyan-ft48-01.lab.bos.redhat.com> (raw)
In-Reply-To: <20081027212638.GA32135@hungry-tiger.westford.ibm.com>

On Mon, Oct 27, 2008 at 05:26:38PM -0400, Michael Meissner wrote:
> On Mon, Oct 27, 2008 at 10:22:42PM +0100, Jakub Jelinek wrote:
> > On Mon, Oct 27, 2008 at 05:13:20PM -0400, Michael Meissner wrote:
> > > However, your test code should declare a to be
> > > signed char instead of char, since there are platforms where chars are unsigned
> > > by default.
> > 
> > I'm aware of that, that's why I've tested the testcase works with both
> > -fsigned-char and -funsigned-char.  I wanted to cover both signed char
> > (on some arches) and unsigned char (on others).
> 
> I was just picking up on whether storing 0x80 would someday cause the test to
> fail on different signed/unsigned char configurations (since 0x80 can't be fit
> in a normal char), and I figured it was better to write type correct code to
> have the meaning clear.

Ok, here is the same patch with testcase that tests both signed char and
unsigned char on all targets.  Bootstrapped/regtested on x86_64-linux, ok
for trunk?

2008-10-28  Jakub Jelinek  <jakub@redhat.com>

	PR c/37924
	* combine.c (make_compound_operation): Don't call make_extraction with
	non-positive length.
	(simplify_shift_const_1): Canonicalize count even if complement_p.

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

--- gcc/combine.c.jj	2008-10-14 12:49:27.000000000 +0200
+++ gcc/combine.c	2008-10-27 19:33:25.000000000 +0100
@@ -6996,7 +6996,8 @@ make_compound_operation (rtx x, enum rtx
       if (GET_CODE (rhs) == CONST_INT
 	  && GET_CODE (lhs) == ASHIFT
 	  && GET_CODE (XEXP (lhs, 1)) == CONST_INT
-	  && INTVAL (rhs) >= INTVAL (XEXP (lhs, 1)))
+	  && INTVAL (rhs) >= INTVAL (XEXP (lhs, 1))
+	  && INTVAL (rhs) < mode_width)
 	{
 	  new_rtx = make_compound_operation (XEXP (lhs, 0), next_code);
 	  new_rtx = make_extraction (mode, new_rtx,
@@ -7016,6 +7017,7 @@ make_compound_operation (rtx x, enum rtx
 		&& (OBJECT_P (SUBREG_REG (lhs))))
 	  && GET_CODE (rhs) == CONST_INT
 	  && INTVAL (rhs) < HOST_BITS_PER_WIDE_INT
+	  && INTVAL (rhs) < mode_width
 	  && (new_rtx = extract_left_shift (lhs, INTVAL (rhs))) != 0)
 	new_rtx = make_extraction (mode, make_compound_operation (new_rtx, next_code),
 			       0, NULL_RTX, mode_width - INTVAL (rhs),
@@ -9003,11 +9005,6 @@ simplify_shift_const_1 (enum rtx_code co
       if (GET_CODE (varop) == CLOBBER)
 	return NULL_RTX;
 
-      /* If we discovered we had to complement VAROP, leave.  Making a NOT
-	 here would cause an infinite loop.  */
-      if (complement_p)
-	break;
-
       /* Convert ROTATERT to ROTATE.  */
       if (code == ROTATERT)
 	{
@@ -9053,6 +9050,11 @@ simplify_shift_const_1 (enum rtx_code co
 	    }
 	}
 
+      /* If we discovered we had to complement VAROP, leave.  Making a NOT
+	 here would cause an infinite loop.  */
+      if (complement_p)
+	break;
+
       /* An arithmetic right shift of a quantity known to be -1 or 0
 	 is a no-op.  */
       if (code == ASHIFTRT
--- gcc/testsuite/gcc.c-torture/execute/pr37924.c.jj	2008-10-27 19:33:25.000000000 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr37924.c	2008-10-28 10:35:39.000000000 +0100
@@ -0,0 +1,50 @@
+/* PR c/37924 */
+
+extern void abort (void);
+
+signed char a;
+unsigned char b;
+
+int
+test1 (void)
+{
+  int c = -1;
+  return ((unsigned int) (a ^ c)) >> 9;
+}
+
+int
+test2 (void)
+{
+  int c = -1;
+  return ((unsigned int) (b ^ c)) >> 9;
+}
+
+int
+main (void)
+{
+  a = 0;
+  if (test1 () != (-1U >> 9))
+    abort ();
+  a = 0x40;
+  if (test1 () != (-1U >> 9))
+    abort ();
+  a = 0x80;
+  if (test1 () != (a < 0) ? 0 : (-1U >> 9))
+    abort ();
+  a = 0xff;
+  if (test1 () != (a < 0) ? 0 : (-1U >> 9))
+    abort ();
+  b = 0;
+  if (test2 () != (-1U >> 9))
+    abort ();
+  b = 0x40;
+  if (test2 () != (-1U >> 9))
+    abort ();
+  b = 0x80;
+  if (test2 () != (-1U >> 9))
+    abort ();
+  b = 0xff;
+  if (test2 () != (-1U >> 9))
+    abort ();
+  return 0;
+}


	Jakub

  reply	other threads:[~2008-10-28 11:27 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-10-27 20:58 [PATCH] Fix combiner issues with shifts (PR c/37924) Jakub Jelinek
2008-10-27 21:54 ` Michael Meissner
2008-10-27 22:30   ` Jakub Jelinek
2008-10-27 22:47     ` Michael Meissner
2008-10-28 12:38       ` Jakub Jelinek [this message]
2008-10-28 21:43         ` [PATCH] Fix combiner issues with shifts (PR c/37924, take 2) Michael Meissner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20081028112718.GZ14706@tyan-ft48-01.lab.bos.redhat.com \
    --to=jakub@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=meissner@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).