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]

RFA: Fix PR42084 (miscompile in RTL expansion)


Hi,

when cleaning up the end of blocks I initially walked until the head of 
the basic block, removing jumps.  That's wrong if those jumps were not 
expanded for the conditional goto but some other gimple statements.  That 
happens e.g. when expanding comparisons on multi word regs.  We must walk 
only up until the instruction before the jump sequence associated with the 
conditional goto.

This fixes the testcase (it breaks only with -m32).  Regstrapping on 
x86_64-linux in progress, okay if that passes?


Ciao,
Michael.
-- 
	PR rtl-optimization/42084
	* cfgexpand.c (maybe_cleanup_end_of_block): Add new parameter,
	use it to stop walking.
	(expand_gimple_cond): Adjust call to above.

testsuite/
	* gcc.dg/pr42084.c: New test.

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 154703)
+++ cfgexpand.c	(working copy)
@@ -1586,10 +1586,11 @@ label_rtx_for_bb (basic_block bb ATTRIBU
 
 /* A subroutine of expand_gimple_cond.  Given E, a fallthrough edge
    of a basic block where we just expanded the conditional at the end,
-   possibly clean up the CFG and instruction sequence.  */
+   possibly clean up the CFG and instruction sequence.  LAST is the
+   last instruction before the just emitted jump sequence.  */
 
 static void
-maybe_cleanup_end_of_block (edge e)
+maybe_cleanup_end_of_block (edge e, rtx last)
 {
   /* Special case: when jumpif decides that the condition is
      trivial it emits an unconditional jump (and the necessary
@@ -1604,7 +1605,6 @@ maybe_cleanup_end_of_block (edge e)
      normally isn't there in a cleaned CFG), fix it here.  */
   if (BARRIER_P (get_last_insn ()))
     {
-      basic_block bb = e->src;
       rtx insn;
       remove_edge (e);
       /* Now, we have a single successor block, if we have insns to
@@ -1620,7 +1620,7 @@ maybe_cleanup_end_of_block (edge e)
       /* Make sure we have an unconditional jump.  Otherwise we're
 	 confused.  */
       gcc_assert (JUMP_P (insn) && !any_condjump_p (insn));
-      for (insn = PREV_INSN (insn); insn != BB_HEAD (bb);)
+      for (insn = PREV_INSN (insn); insn != last;)
 	{
 	  insn = PREV_INSN (insn);
 	  if (JUMP_P (NEXT_INSN (insn)))
@@ -1699,7 +1699,7 @@ expand_gimple_cond (basic_block bb, gimp
 	}
       true_edge->goto_block = NULL;
       false_edge->flags |= EDGE_FALLTHRU;
-      maybe_cleanup_end_of_block (false_edge);
+      maybe_cleanup_end_of_block (false_edge, last);
       return NULL;
     }
   if (true_edge->dest == bb->next_bb)
@@ -1715,7 +1715,7 @@ expand_gimple_cond (basic_block bb, gimp
 	}
       false_edge->goto_block = NULL;
       true_edge->flags |= EDGE_FALLTHRU;
-      maybe_cleanup_end_of_block (true_edge);
+      maybe_cleanup_end_of_block (true_edge, last);
       return NULL;
     }
 
Index: testsuite/gcc.dg/pr42084.c
===================================================================
--- testsuite/gcc.dg/pr42084.c	(revision 0)
+++ testsuite/gcc.dg/pr42084.c	(revision 0)
@@ -0,0 +1,22 @@
+/* { dg-do run } */
+/* { dg-options "-O1 -fno-delete-null-pointer-checks" } */
+extern void abort (void);
+int g = 0;
+static int __attribute__((noinline)) f (long long a, long long b)
+{
+  int cmp;
+  cmp = a > b;
+  if (&g == 0)
+    cmp-=2;
+  else
+    cmp++;
+  return cmp;
+}
+
+int main (void)
+{
+  int ret = f (2, 1);
+  if (ret != 2)
+    abort ();
+  return 0;
+}


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