Re: [patch][4.5] Make regmove cfglayout-safe

[Paolo Bonzini <bonzini at gnu dot org>]

Paolo Bonzini wrote:
>> I also wondered about this.  I think the original idea is that splits
>> can call into dojump.c.
> 
> A more likely possibility is -fnon-call-exceptions.

Of course this is the main cause.  But splitting one jump to multiple
jumps is supported and actually even documented.  It will happen for
example in this testcase:

int f(float x) { if (x != x) return 5; else abort (); }


on i386 which produces

        fucomip %st(0), %st
        jp      .L8
        je      .L6

It is possible to change this to an expander in the i386 md of course.
I don't think any other backend is relying on it, but I will make a more
thorough check if I end up submitting something like the attached patch.

Paolo

2009-03-10  Paolo Bonzini  <bonzini@gnu.org>

	* lower-subreg.c (decompose_multiword_subregs): Extract code...
	* cfgbuild.c (rtl_split_blocks_for_eh): ... here.
	* basic-block.h (rtl_split_blocks_for_eh): Declare it.

	* recog.c (split_insn): Return bool.  Check that the splitter
	produces no barriers and no labels.
	(split_all_insns): Use the result.  Call rtl_split_blocks_for_eh
	instead of find_many_sub_basic_blocks.
	* reload1.c (fixup_abnormal_edges): Use it.
	* passes.c (init_optimization_passes): Move cfglayout mode
	further down.

Index: gcc/passes.c
===================================================================
--- gcc/passes.c	(branch combine-cfglayout)
+++ gcc/passes.c	(working copy)
@@ -757,8 +757,8 @@ init_optimization_passes (void)
       NEXT_PASS (pass_if_after_combine);
       NEXT_PASS (pass_partition_blocks);
       NEXT_PASS (pass_regmove);
-      NEXT_PASS (pass_outof_cfg_layout_mode);
       NEXT_PASS (pass_split_all_insns);
+      NEXT_PASS (pass_outof_cfg_layout_mode);
       NEXT_PASS (pass_lower_subreg2);
       NEXT_PASS (pass_df_initialize_no_opt);
       NEXT_PASS (pass_stack_ptr_mod);
Index: gcc/recog.c
===================================================================
--- gcc/recog.c	(branch combine-cfglayout)
+++ gcc/recog.c	(working copy)
@@ -29,6 +29,7 @@ along with GCC; see the file COPYING3.  
 #include "insn-config.h"
 #include "insn-attr.h"
 #include "hard-reg-set.h"
+#include "except.h"
 #include "recog.h"
 #include "regs.h"
 #include "addresses.h"
@@ -71,7 +72,6 @@ get_attr_enabled (rtx insn ATTRIBUTE_UNU
 
 static void validate_replace_rtx_1 (rtx *, rtx, rtx, rtx, bool);
 static void validate_replace_src_1 (rtx *, void *);
-static rtx split_insn (rtx);
 
 /* Nonzero means allow operands to be volatile.
    This should be 0 if you are generating rtl, such as if you are calling
@@ -2671,19 +2671,23 @@ reg_fits_class_p (rtx operand, enum reg_
 }
 
 /* Split single instruction.  Helper function for split_all_insns and
-   split_all_insns_noflow.  Return last insn in the sequence if successful,
-   or NULL if unsuccessful.  */
+   split_all_insns_noflow.  Return whether new control flow insns
+   were added.  */
 
-static rtx
+static bool
 split_insn (rtx insn)
 {
   /* Split insns here to get max fine-grain parallelism.  */
   rtx first = PREV_INSN (insn);
   rtx last = try_split (PATTERN (insn), insn, 1);
   rtx insn_set, last_set, note;
+  bool new_cfi = false;
+  bool was_cfi;
 
   if (last == insn)
-    return NULL_RTX;
+    return false;
+
+  was_cfi = control_flow_insn_p (insn);
 
   /* If the original instruction was a single set that was known to be
      equivalent to a constant, see if we can say the same about the last
@@ -2706,22 +2710,25 @@ split_insn (rtx insn)
   /* try_split returns the NOTE that INSN became.  */
   SET_INSN_DELETED (insn);
 
-  /* ??? Coddle to md files that generate subregs in post-reload
-     splitters instead of computing the proper hard register.  */
-  if (reload_completed && first != last)
+  while (first != last)
     {
       first = NEXT_INSN (first);
-      for (;;)
+      gcc_assert (!BARRIER_P (first) && !LABEL_P (first));
+
+      /* ??? Coddle to md files that generate subregs in post-reload
+         splitters instead of computing the proper hard register.  */
+      if (reload_completed && INSN_P (first))
+	cleanup_subreg_operands (first);
+      if ((first != last || !was_cfi)
+          && control_flow_insn_p (first))
 	{
-	  if (INSN_P (first))
-	    cleanup_subreg_operands (first);
-	  if (first == last)
-	    break;
-	  first = NEXT_INSN (first);
+	  gcc_assert (flag_non_call_exceptions
+		      && can_throw_internal (first));
+	  new_cfi = true;
 	}
     }
 
-  return last;
+  return new_cfi;
 }
 
 /* Split all insns in the function.  If UPD_LIFE, update life info after.  */
@@ -2730,12 +2737,10 @@ void
 split_all_insns (void)
 {
   sbitmap blocks;
-  bool changed;
   basic_block bb;
 
   blocks = sbitmap_alloc (last_basic_block);
   sbitmap_zero (blocks);
-  changed = false;
 
   FOR_EACH_BB_REVERSE (bb)
     {
@@ -2753,41 +2758,17 @@ split_all_insns (void)
 	    {
 	      rtx set = single_set (insn);
 
-	      /* Don't split no-op move insns.  These should silently
-		 disappear later in final.  Splitting such insns would
-		 break the code that handles LIBCALL blocks.  */
 	      if (set && set_noop_p (set))
-		{
-		  /* Nops get in the way while scheduling, so delete them
-		     now if register allocation has already been done.  It
-		     is too risky to try to do this before register
-		     allocation, and there are unlikely to be very many
-		     nops then anyways.  */
-		  if (reload_completed)
-		      delete_insn_and_edges (insn);
-		}
-	      else
-		{
-		  rtx last = split_insn (insn);
-		  if (last)
-		    {
-		      /* The split sequence may include barrier, but the
-			 BB boundary we are interested in will be set to
-			 previous one.  */
-
-		      while (BARRIER_P (last))
-			last = PREV_INSN (last);
-		      SET_BIT (blocks, bb->index);
-		      changed = true;
-		    }
-		}
+		delete_insn_and_edges (insn);
+
+	      else if (split_insn (insn))
+		SET_BIT (blocks, bb->index);
 	    }
 	}
     }
 
   default_rtl_profile ();
-  if (changed)
-    find_many_sub_basic_blocks (blocks);
+  rtl_split_blocks_for_eh (blocks, true);
 
 #ifdef ENABLE_CHECKING
   verify_flow_info ();
Index: gcc/lower-subreg.c
===================================================================
--- gcc/lower-subreg.c	(branch combine-cfglayout)
+++ gcc/lower-subreg.c	(working copy)
@@ -1139,8 +1139,6 @@ decompose_multiword_subregs (void)
   if (!bitmap_empty_p (decomposable_context))
     {
       sbitmap sub_blocks;
-      unsigned int i;
-      sbitmap_iterator sbi;
       bitmap_iterator iter;
       unsigned int regno;
 
@@ -1248,35 +1246,10 @@ decompose_multiword_subregs (void)
 	}
 
       /* If we had insns to split that caused control flow insns in the middle
-	 of a basic block, split those blocks now.  Note that we only handle
+	 of a basic block, split those blocks now.  We only have to handle
 	 the case where splitting a load has caused multiple possibly trapping
 	 loads to appear.  */
-      EXECUTE_IF_SET_IN_SBITMAP (sub_blocks, 0, i, sbi)
-	{
-	  rtx insn, end;
-	  edge fallthru;
-
-	  bb = BASIC_BLOCK (i);
-	  insn = BB_HEAD (bb);
-	  end = BB_END (bb);
-
-	  while (insn != end)
-	    {
-	      if (control_flow_insn_p (insn))
-		{
-		  /* Split the block after insn.  There will be a fallthru
-		     edge, which is OK so we keep it.  We have to create the
-		     exception edges ourselves.  */
-		  fallthru = split_block (bb, insn);
-		  rtl_make_eh_edge (NULL, bb, BB_END (bb));
-		  bb = fallthru->dest;
-		  insn = BB_HEAD (bb);
-		}
-	      else
-	        insn = NEXT_INSN (insn);
-	    }
-	}
-
+      rtl_split_blocks_for_eh (sub_blocks, true);
       sbitmap_free (sub_blocks);
     }
 
Index: gcc/cfgbuild.c
===================================================================
--- gcc/cfgbuild.c	(branch combine-cfglayout)
+++ gcc/cfgbuild.c	(working copy)
@@ -216,6 +216,53 @@ rtl_make_eh_edge (sbitmap edge_cache, ba
   free_INSN_LIST_list (&handlers);
 }
 
+/* Look in BLOCKS for control flow insns in the middle of the basic block,
+   typically caused by splitting insns.  The pass is skipped except for
+   -fnon-call-exceptions if NON_CALL_EXCEPTIONS_ONLY true.  */
+void
+rtl_split_blocks_for_eh (sbitmap blocks, bool non_call_exceptions_only)
+{
+  unsigned int i;
+  sbitmap_iterator sbi;
+
+#ifndef ENABLE_CHECKING
+  if (non_call_exceptions_only && !flag_non_call_exceptions)
+    return;
+#endif
+
+  EXECUTE_IF_SET_IN_SBITMAP (blocks, 0, i, sbi)
+    {
+      rtx insn, end;
+      edge fallthru;
+      basic_block bb;
+
+      bb = BASIC_BLOCK (i);
+      insn = BB_HEAD (bb);
+      end = BB_END (bb);
+
+      while (insn != end)
+	{
+	  if (control_flow_insn_p (insn))
+	    {
+#ifdef ENABLE_CHECKING
+	      gcc_assert (!non_call_exceptions_only
+			  || (flag_non_call_exceptions
+			      && can_throw_internal (insn)));
+#endif
+	      /* Split the block after insn.  There will be a fallthru
+		 edge, which is OK so we keep it.  We have to create the
+		 exception edges ourselves.  */
+	      fallthru = split_block (bb, insn);
+	      rtl_make_eh_edge (NULL, bb, BB_END (bb));
+	      bb = fallthru->dest;
+	      insn = BB_HEAD (bb);
+	    }
+	  else
+	    insn = NEXT_INSN (insn);
+	}
+    }
+}
+
 /* States of basic block as seen by find_many_sub_basic_blocks.  */
 enum state {
   /* Basic blocks created via split_block belong to this state.
Index: gcc/basic-block.h
===================================================================
--- gcc/basic-block.h	(branch combine-cfglayout)
+++ gcc/basic-block.h	(working copy)
@@ -896,6 +896,7 @@ extern bool purge_dead_edges (basic_bloc
 /* In cfgbuild.c.  */
 extern void find_many_sub_basic_blocks (sbitmap);
 extern void rtl_make_eh_edge (sbitmap, basic_block, rtx);
+extern void rtl_split_blocks_for_eh (sbitmap, bool);
 extern void find_basic_blocks (rtx);
 
 /* In cfgcleanup.c.  */
Index: gcc/reload1.c
===================================================================
--- gcc/reload1.c	(branch combine-cfglayout)
+++ gcc/reload1.c	(working copy)
@@ -8925,7 +8925,7 @@ fixup_abnormal_edges (void)
       sbitmap blocks;
       blocks = sbitmap_alloc (last_basic_block);
       sbitmap_ones (blocks);
-      find_many_sub_basic_blocks (blocks);
+      rtl_split_blocks_for_eh (blocks, true);
       sbitmap_free (blocks);
     }