[patch] Fix PR40021: miscompile of BLAS (hence tonto and gamess)

Michael Matz matz@suse.de
Wed May 6 16:00:00 GMT 2009


Hi,

as detailed in the bugreport we can end up in a situation where a basic 
block with just one successor edge ends with multiple jump instructions to 
the same target (only during expansion itself).  We are confused if we 
want to insert instructions on such edge and insert them before the last 
jump, but after the others, which later leads to miscompilations as the 
inserted instruction isn't ru then.

This fixes the miscompile (with test input and of the testcase).  
Regstrapped on x86_64-linux.  Okay?


Ciao,
Michael.
-- 
	PR middle-end/40021
	* cfgexpand.c (maybe_cleanup_end_of_block): New static function.
	(expand_gimple_cond): Use it to cleanup CFG and superfluous jumps.

Index: cfgexpand.c
===================================================================
--- cfgexpand.c	(revision 147126)
+++ cfgexpand.c	(working copy)
@@ -1724,6 +1724,52 @@ 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.  */
+
+static void
+maybe_cleanup_end_of_block (edge e)
+{
+  basic_block bb = e->src;
+  /* Special case: when jumpif decides that the condition is
+     trivial it emits an unconditional jump (and the necessary
+     barrier).  But we still have two edges, the fallthru one is
+     wrong.  purge_dead_edges would clean this up later.  Unfortunately
+     we have to insert insns (and split edges) before
+     find_many_sub_basic_blocks and hence before purge_dead_edges.
+     But splitting edges might create new blocks which depend on the
+     fact that if there are two edges there's no barrier.  So the
+     barrier would get lost and verify_flow_info would ICE.  Instead
+     of auditing all edge splitters to care for the barrier (which
+     normally isn't there in a cleaned CFG), fix it here.  */
+  if (BARRIER_P (get_last_insn ()))
+    {
+      rtx insn;
+      remove_edge (e);
+      /* Now, we have a single successor block, if we have insns to
+	 insert on the remaining edge we potentially will insert
+	 it at the end of this block (if the dest block isn't feasible)
+	 in order to avoid splitting the edge.  This insertion will take
+	 place in front of the last jump.  But we might have emitted
+	 multiple jumps (conditional and one unconditional) to the
+	 same destination.  Inserting in front of the last one then
+	 is a problem.  See PR 40021.  We fix this by deleting all
+	 jumps except the last unconditional one.  */
+      insn = PREV_INSN (get_last_insn ());
+      /* 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);)
+	{
+	  insn = PREV_INSN (insn);
+	  if (JUMP_P (NEXT_INSN (insn)))
+	    delete_insn (NEXT_INSN (insn));
+	}
+    }
+}
+
+
 /* A subroutine of expand_gimple_basic_block.  Expand one GIMPLE_COND.
    Returns a new basic block if we've terminated the current basic
    block and created a new one.  */
@@ -1767,19 +1813,7 @@ expand_gimple_cond (basic_block bb, gimp
       true_edge->goto_block = NULL;
       false_edge->flags |= EDGE_FALLTHRU;
       ggc_free (pred);
-      /* Special case: when jumpif decides that the condition is
-         trivial it emits an unconditional jump (and the necessary
-	 barrier).  But we still have two edges, the fallthru one is
-	 wrong.  purge_dead_edges would clean this up later.  Unfortunately
-	 we have to insert insns (and split edges) before
-	 find_many_sub_basic_blocks and hence before purge_dead_edges.
-	 But splitting edges might create new blocks which depend on the
-	 fact that if there are two edges there's no barrier.  So the
-	 barrier would get lost and verify_flow_info would ICE.  Instead
-	 of auditing all edge splitters to care for the barrier (which
-	 normally isn't there in a cleaned CFG), fix it here.  */
-      if (BARRIER_P (get_last_insn ()))
-	remove_edge (false_edge);
+      maybe_cleanup_end_of_block (false_edge);
       return NULL;
     }
   if (true_edge->dest == bb->next_bb)
@@ -1796,8 +1830,7 @@ expand_gimple_cond (basic_block bb, gimp
       false_edge->goto_block = NULL;
       true_edge->flags |= EDGE_FALLTHRU;
       ggc_free (pred);
-      if (BARRIER_P (get_last_insn ()))
-	remove_edge (true_edge);
+      maybe_cleanup_end_of_block (true_edge);
       return NULL;
     }
 



More information about the Gcc-patches mailing list