This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PR middle-end/20491] combine generates bad subregs


On Mar 28, 2005, Richard Henderson <rth@gcc.gnu.org> wrote:

> On Thu, Mar 24, 2005 at 07:45:44AM -0300, Alexandre Oliva wrote:
>> * combine.c (subst): Make sure we don't create invalid subregs.

> Ok.

As it turned out, the bug seems to have been fixed by some other
patch, because I no longer get the error in mainline or in the 4.0
branch.  Since my patch actually introduced a regression on
i686-pc-linux-gnu: gcc.dg/i386-rotate-1.c.  Because of the ruled-out
combines, we prevented the complete transformation from taking place.
I actually wrote another, more complex patch, that fixed it, by using
the same machinery used by later portions of combine to push the
invalid subreg into the shift operands, i.e., turn (subreg (ashift
(reg) (const_int))) into (ashift (subreg (reg)) (const_int)), but then
I decided I was working too hard and figured I might be better off
fixing the problem elsewhere.  To my surprise, after reverting the
patch I had, the problem no longer occurred.  I tried some CVS
searching, but couldn't locate the patch that fixed the problem.  In
fact, I couldn't even find a relatively-recent tree that triggered the
bug, although I'm pretty sure the tree in which I triggered the bug
was no older than some date early last week.  Oh well...

I'm attaching the patch I had, in case you still think we should avoid
creating such invalid subregs, followed by the testcase patch I
installed in the 4.0 branch in mainline.

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR middle-end/20491
	* combine.c (subst): Make sure we don't create invalid subregs.

Index: gcc/combine.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/combine.c,v
retrieving revision 1.484
diff -u -p -r1.484 combine.c
--- gcc/combine.c 22 Mar 2005 03:48:44 -0000 1.484
+++ combine.c	25 Mar 2005 21:21:40 -0000
@@ -3689,15 +3689,36 @@ subst (rtx x, rtx from, rtx to, int in_d
 	      if (GET_CODE (new) == CLOBBER && XEXP (new, 0) == const0_rtx)
 		return new;
 
+	      /* If we changed the reg of a subreg, make sure it's
+		 still valid.  For anything but a REG, require the
+		 SUBREG to be simplified out.  */
+
 	      if (GET_CODE (x) == SUBREG
-		  && (GET_CODE (new) == CONST_INT
-		      || GET_CODE (new) == CONST_DOUBLE))
+		  && ! rtx_equal_p (new, SUBREG_REG (x)))
 		{
 		  enum machine_mode mode = GET_MODE (x);
+		  rtx orig = x;
+
+		  x = simplify_gen_subreg (mode, new, op0_mode,
+					   SUBREG_BYTE (orig));
+
+		  /* This will propagate the subreg into the operand,
+		     if appropriate.  */
+		  if (GET_CODE (x) == SUBREG
+		      && GET_CODE (SUBREG_REG (x)) != REG)
+		    x = force_to_mode (x, mode, GET_MODE_MASK (mode),
+				       NULL_RTX, 0);
+
+		  /* Now make sure we didn't create a subreg of
+		     something that is not a reg.  */
+		  if (GET_CODE (x) == SUBREG
+		      && GET_CODE (SUBREG_REG (x)) != REG)
+		    x = simplify_subreg (mode, SUBREG_REG (x),
+					 GET_MODE (SUBREG_REG (x)),
+					 SUBREG_BYTE (x));
+
+		  op0_mode = VOIDmode;
 
-		  x = simplify_subreg (GET_MODE (x), new,
-				       GET_MODE (SUBREG_REG (x)),
-				       SUBREG_BYTE (x));
 		  if (! x)
 		    x = gen_rtx_CLOBBER (mode, const0_rtx);
 		}
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR middle-end/20491
	* gcc.dg/torture/asm-subreg-1.c: New test.

Index: gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
===================================================================
RCS file: gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
diff -N gcc/testsuite/gcc.dg/torture/asm-subreg-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/gcc.dg/torture/asm-subreg-1.c 29 Mar 2005 21:31:55 -0000
@@ -0,0 +1,14 @@
+/* PR middle-end/20491 */
+
+/* { dg-do compile } */
+
+/* Combine used to introduce invalid subregs for the asm input, and
+   we'd crash later on, when removing all subregs.  */
+
+volatile unsigned short _const_32 [4] = {1,2,3,4};
+void
+evas_common_convert_yuv_420p_601_rgba()
+{
+  __asm__ __volatile__ ("" : : "X" (*_const_32));
+}
+

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]