[PATCH] Improvements to -freorder-blocks-and-partition support

Teresa Johnson tejohnson@google.com
Tue May 14 21:42:00 GMT 2013


Patch 1 of 3 split out from the patch I sent last week that fixes problems with
-freorder-blocks-and-partition, with some additional verification improvements.

See http://gcc.gnu.org/ml/gcc-patches/2013-05/threads.html#00388 for context.

This patch adds a flag to the rtl_data structure to indicate whether any
partitioning was actually performed, so that optimizations which were
conservatively disabled whenever the flag_reorder_blocks_and_partition
is enabled (e.g. try_crossjump_to_edge, part of connect_traces) can be less
conservative for functions where no partitions were formed (e.g. they are
completely hot).

It also adds another flag to the rtl_data structure to indicate whether bb
reordering is complete, and if so enables sanity checking that there is
at most one transition in the layout order between hot and cold sections.
This was moved from verify_hot_cold_block_grouping, which was only called
once at the end of the bbro pass and is now removed.

Tested on x86_64-unknown-linux-gnu with bootstrap and profiledbootstrap
builds and regression testing. Additionally built/ran cpu2006 with profile
feedback and -freorder-blocks-and-partition enabled (which currently has
build failures from splitting until my follow-on patches are in, but confirmed
no new failures due to this patch), as well as gcc regression testing with
-freorder-blocks-and-partition enabled.

Ok for trunk?

Thanks,
Teresa

2013-05-14  Teresa Johnson  <tejohnson@google.com>

	* function.h (has_bb_partition): New rtl_data flag.
        (bb_reorder_complete): Ditto.
        * cfgrtl.c (rtl_verify_flow_info_1): After bbro, verify
        that text sections switch at most once in layout order.
	* bb-reorder.c (connect_traces): Check for has_bb_partition
        instead of flag_reorder_blocks_and_partition.
	(verify_hot_cold_block_grouping): Remove.
	(reorder_basic_blocks): Remove call to deleted
        verify_hot_cold_block_grouping, and set bb_reorder_complete.
	(partition_hot_cold_basic_blocks): Set has_bb_partition.
	* cfgcleanup.c (try_crossjump_to_edge): Check for has_bb_partition
        instead of flag_reorder_blocks_and_partition.

Index: bb-reorder.c
===================================================================
--- bb-reorder.c	(revision 198891)
+++ bb-reorder.c	(working copy)
@@ -1053,7 +1053,7 @@ connect_traces (int n_traces, struct trace *traces
   current_partition = BB_PARTITION (traces[0].first);
   two_passes = false;
 
-  if (flag_reorder_blocks_and_partition)
+  if (crtl->has_bb_partition)
     for (i = 0; i < n_traces && !two_passes; i++)
       if (BB_PARTITION (traces[0].first)
 	  != BB_PARTITION (traces[i].first))
@@ -1262,7 +1262,7 @@ connect_traces (int n_traces, struct trace *traces
 		      }
 		  }
 
-	      if (flag_reorder_blocks_and_partition)
+	      if (crtl->has_bb_partition)
 		try_copy = false;
 
 	      /* Copy tiny blocks always; copy larger blocks only when the
@@ -2068,43 +2068,6 @@ add_reg_crossing_jump_notes (void)
 	add_reg_note (BB_END (e->src), REG_CROSSING_JUMP, NULL_RTX);
 }
 
-/* Verify, in the basic block chain, that there is at most one switch
-   between hot/cold partitions. This is modelled on
-   rtl_verify_flow_info_1, but it cannot go inside that function
-   because this condition will not be true until after
-   reorder_basic_blocks is called.  */
-
-static void
-verify_hot_cold_block_grouping (void)
-{
-  basic_block bb;
-  int err = 0;
-  bool switched_sections = false;
-  int current_partition = 0;
-
-  FOR_EACH_BB (bb)
-    {
-      if (!current_partition)
-	current_partition = BB_PARTITION (bb);
-      if (BB_PARTITION (bb) != current_partition)
-	{
-	  if (switched_sections)
-	    {
-	      error ("multiple hot/cold transitions found (bb %i)",
-		     bb->index);
-	      err = 1;
-	    }
-	  else
-	    {
-	      switched_sections = true;
-	      current_partition = BB_PARTITION (bb);
-	    }
-	}
-    }
-
-  gcc_assert(!err);
-}
-
 /* Reorder basic blocks.  The main entry point to this file.  FLAGS is
    the set of flags to pass to cfg_layout_initialize().  */
 
@@ -2157,8 +2120,9 @@ reorder_basic_blocks (void)
       dump_flow_info (dump_file, dump_flags);
     }
 
-  if (flag_reorder_blocks_and_partition)
-    verify_hot_cold_block_grouping ();
+  /* Signal that rtl_verify_flow_info_1 can now verify that there
+     is at most one switch between hot/cold sections.  */
+  crtl->bb_reorder_complete = true;
 }
 
 /* Determine which partition the first basic block in the function
@@ -2503,6 +2467,8 @@ partition_hot_cold_basic_blocks (void)
   if (!crossing_edges.exists ())
     return 0;
 
+  crtl->has_bb_partition = true;
+
   /* Make sure the source of any crossing edge ends in a jump and the
      destination of any crossing edge has a label.  */
   add_labels_and_missing_jumps (crossing_edges);
Index: cfgrtl.c
===================================================================
--- cfgrtl.c	(revision 198891)
+++ cfgrtl.c	(working copy)
@@ -2082,6 +2082,8 @@ rtl_verify_flow_info_1 (void)
   rtx x;
   int err = 0;
   basic_block bb;
+  bool switched_sections = false;
+  int current_partition = BB_UNPARTITIONED;
 
   /* Check the general integrity of the basic blocks.  */
   FOR_EACH_BB_REVERSE (bb)
@@ -2299,6 +2301,29 @@ rtl_verify_flow_info_1 (void)
 	  err = 1;
 	}
 
+      /* Verify, in the basic block chain, that there is at most one switch
+         between hot/cold partitions. This condition will not be true until
+         after reorder_basic_blocks is called.  */
+      if (crtl->bb_reorder_complete)
+        {
+          if (current_partition != BB_UNPARTITIONED
+              && BB_PARTITION (bb) != current_partition)
+            {
+              if (switched_sections)
+              {
+                error ("multiple hot/cold transitions found (bb %i)",
+                       bb->index);
+                err = 1;
+              }
+              else
+                switched_sections = true;
+
+              if (!crtl->has_bb_partition)
+                error ("partition found but function partition flag not set");
+            }
+          current_partition = BB_PARTITION (bb);
+        }
+
       if (BB_END (bb) == x)
 	/* Do checks for empty blocks here.  */
 	;
Index: function.h
===================================================================
--- function.h	(revision 198891)
+++ function.h	(working copy)
@@ -446,6 +446,15 @@ struct GTY(()) rtl_data {
      sched2) and is useful only if the port defines LEAF_REGISTERS.  */
   bool uses_only_leaf_regs;
 
+  /* Nonzero if the function being compiled has undergone hot/cold partitioning
+     (under flag_reorder_blocks_and_partition) and has at least one cold
+     block.  */
+  bool has_bb_partition;
+
+  /* Nonzero if the function being compiled has completed the bb reordering
+     pass.  */
+  bool bb_reorder_complete;
+
   /* Like regs_ever_live, but 1 if a reg is set or clobbered from an
      asm.  Unlike regs_ever_live, elements of this array corresponding
      to eliminable regs (like the frame pointer) are set if an asm
Index: cfgcleanup.c
===================================================================
--- cfgcleanup.c	(revision 198891)
+++ cfgcleanup.c	(working copy)
@@ -1864,7 +1864,7 @@ try_crossjump_to_edge (int mode, edge e1, edge e2,
      partition boundaries).  See the comments at the top of
      bb-reorder.c:partition_hot_cold_basic_blocks for complete details.  */
 
-  if (flag_reorder_blocks_and_partition && reload_completed)
+  if (crtl->has_bb_partition && reload_completed)
     return false;
 
   /* Search backward through forwarder blocks.  We don't need to worry



More information about the Gcc-patches mailing list