This is the mail archive of the gcc-bugs@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]

[Bug middle-end/18628] [4.0/4.1 regression] miscompilation of switch statement in loop


------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-09 10:30 -------
Subject: [PR middle-end/18628] do not fold to label load from tablejump to reg

This patch is meant to implement suggestion #3 proposed to fix the bug
by Roger Sayle and selected by RTH in bugzilla.  So far, I've only
verified that it fixes the testcase included in the patch.

The thought of adding the REG_EQUAL note was to help other passes that
might want to turn the indirect jump into a direct jump.  I'm not sure
this may actually happen.

Bootstrap and regtesting starting shortly.  Ok to install if they
pass?

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

	PR middle-end/18628
	* cse.c (fold_rtx_mem): Instead of returning the label extracted
	from a tablejump, add it as an REG_EQUAL note, if the insn loaded
	from the table to a register.
	(cse_insn): Don't use it as src_eqv.

Index: gcc/cse.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cse.c,v
retrieving revision 1.349
diff -u -p -r1.349 cse.c
--- gcc/cse.c 8 Mar 2005 13:56:56 -0000 1.349
+++ gcc/cse.c 9 Mar 2005 10:19:13 -0000
@@ -3515,8 +3515,36 @@ fold_rtx_mem (rtx x, rtx insn)
 	    if (offset >= 0
 		&& (offset / GET_MODE_SIZE (GET_MODE (table))
 		    < XVECLEN (table, 0)))
-	      return XVECEXP (table, 0,
-			      offset / GET_MODE_SIZE (GET_MODE (table)));
+	      {
+		rtx label = XVECEXP
+		  (table, 0, offset / GET_MODE_SIZE (GET_MODE (table)));
+		rtx set;
+
+		/* If we have an insn that loads the label from the
+		   jumptable into a reg, we don't want to set the reg
+		   to the label, because this may cause a reference to
+		   the label to remain after the label is removed in
+		   some very obscure cases (PR middle-end/18628).  So
+		   we just set a REG_EQUAL note for this case, and
+		   return the original MEM.  */
+		if (!insn)
+		  return label;
+
+		set = single_set (insn);		
+
+		if (! set || SET_SRC (set) != x)
+		  return x;
+
+		/* If it's a jump, it's safe to reference the label.  */
+		if (SET_DEST (set) == pc_rtx)
+		  return label;
+
+		/* If it's not a REG, the REG_EQUAL note is inappropriate.  */
+		if (REG_P (SET_DEST (set)))
+		  set_unique_reg_note (insn, REG_EQUAL, label);
+
+		return x;
+	      }
 	  }
 	if (table_insn && JUMP_P (table_insn)
 	    && GET_CODE (PATTERN (table_insn)) == ADDR_DIFF_VEC)
@@ -4861,6 +4889,13 @@ cse_insn (rtx insn, rtx libcall_insn)
     {
       src_eqv = fold_rtx (canon_reg (XEXP (tem, 0), NULL_RTX), insn);
       XEXP (tem, 0) = src_eqv;
+
+      /* We don't want to use the labels in REG_EQUAL notes that
+	 fold_rtx may have added in an earlier pass.  If it's
+	 something as simple as a LABEL_REF and we didn't set to it
+	 directly, there was a reason not to do so.  */
+      if (GET_CODE (src_eqv) == LABEL_REF)
+	src_eqv = NULL;
     }
 
   /* Canonicalize sources and addresses of destinations.
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.dg/pr18628.c: New.

Index: gcc/testsuite/gcc.dg/pr18628.c
===================================================================
RCS file: gcc/testsuite/gcc.dg/pr18628.c
diff -N gcc/testsuite/gcc.dg/pr18628.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/gcc.dg/pr18628.c 9 Mar 2005 10:19:27 -0000
@@ -0,0 +1,31 @@
+/* { dg-do link } */
+/* { dg-options "-O2" } */
+
+/* PR middle-end/18628 exposed a problem in which cse folded a load
+   from a jump table into the label that was the target of the branch.
+   Unfortunately, the indirect jump was moved to a different basic
+   block, and the LABEL_REF copied to the register wasn't enough to
+   keep the cfg from optimizing the otherwise-unused label away.  So
+   we ended up with a dangling reference to the label.  */
+
+int i;
+
+int main()
+{
+  for (;;)
+  {
+    switch (i)
+    {
+      case 0:
+      case 1:
+        return 1;
+
+      case 2:
+      case 3:
+        return 0;
+
+      case 5:
+        --i;
+    }
+  }
+}

-- 
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}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=18628


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