[PATCH][RFC] Add bitmap_clear_first_set_bit for worklist handling

Richard Biener rguenther@suse.de
Tue Oct 16 09:17:00 GMT 2018


This adds bitmap_clear_first_set_bit to replace the common

  unsigned idx = bitmap_first_set_bit (b);
  bitmap_clear_bit (b, idx);

pattern which disturbs the bitmap lookup cached element.  We can
always find and clear the first set bit in O(1) so this implements
this as a primitive.

Alternatively one could make sure to never be equal to first
(but for the only element -- 'current' is a bit weird if you look at 
bitmap_find_bit).  That would make clearing the first bit never
update ->current.

For the weird testcase in PR63155 this makes a noticable difference
(~5% compile-time).

Thoughts?

Thanks,
Richard.

2018-10-16  Richard Biener  <rguenther@suse.de>

	* bitmap.h (bitmap_clear_first_set_bit): Declare.
	* bitmap.c (bitmap_first_set_bit): Make a wrapper around ...
	(bitmap_first_set_bit_1): ... this worker that now also can
	clear the bit.
	(bitmap_clear_first_set_bit): New.
	* tree-ssa-propagate.c (ssa_propagation_engine::simulate_stmt):
	Clear stmt from the worklist in callers.
	(ssa_propagation_engine::simulate_block): Here.
	(ssa_propagation_engine::ssa_propagate): Use
	bitmap_clear_first_set_bit to pop off worklist items.
	* tree-ssa-sccvn.c (do_rpo_vn): Likewise.
	* tree-ssa-dce.c (simple_dce_from_worklist): Likewise.
	* tree-cfgcleanup.c (cleanup_tree_cfg_noloop): Likewise.

Index: gcc/bitmap.c
===================================================================
--- gcc/bitmap.c	(revision 265185)
+++ gcc/bitmap.c	(working copy)
@@ -771,12 +771,12 @@ bitmap_single_bit_set_p (const_bitmap a)
 
 
 /* Return the bit number of the first set bit in the bitmap.  The
-   bitmap must be non-empty.  */
+   bitmap must be non-empty.  Clear the bit if clear_p is true.  */
 
-unsigned
-bitmap_first_set_bit (const_bitmap a)
+static unsigned
+bitmap_first_set_bit_1 (bitmap a, bool clear_p)
 {
-  const bitmap_element *elt = a->first;
+  bitmap_element *elt = a->first;
   unsigned bit_no;
   BITMAP_WORD word;
   unsigned ix;
@@ -818,9 +818,29 @@ bitmap_first_set_bit (const_bitmap a)
 
  gcc_checking_assert (word & 1);
 #endif
+ if (clear_p)
+   {
+     BITMAP_WORD bit_val = ((BITMAP_WORD) 1) << (bit_no % BITMAP_WORD_BITS);
+     elt->bits[ix] &= ~bit_val;
+     if (!elt->bits[ix]
+	 && bitmap_element_zerop (elt))
+       bitmap_element_free (a, elt);
+   }
  return bit_no;
 }
 
+unsigned
+bitmap_clear_first_set_bit (bitmap a)
+{
+  return bitmap_first_set_bit_1 (a, true);
+}
+
+unsigned
+bitmap_first_set_bit (const_bitmap a)
+{
+  return bitmap_first_set_bit_1 (const_cast<bitmap>(a), false);
+}
+
 /* Return the bit number of the first set bit in the bitmap.  The
    bitmap must be non-empty.  */
 
Index: gcc/bitmap.h
===================================================================
--- gcc/bitmap.h	(revision 265185)
+++ gcc/bitmap.h	(working copy)
@@ -52,9 +52,10 @@ along with GCC; see the file COPYING3.
 
    The following operations can always be performed in O(1) time:
 
+     * empty_p			: bitmap_empty_p
      * clear			: bitmap_clear
-     * choose_one		: (not implemented, but could be
-				   implemented in constant time)
+     * choose_one		: bitmap_first_set_bit
+     * remove_one		: bitmap_clear_first_set_bit
 
    The following operations can be performed in O(E) time worst-case (with
    E the number of elements in the linked list), but in O(1) time with a
@@ -359,6 +360,7 @@ extern void debug (const bitmap_head &re
 extern void debug (const bitmap_head *ptr);
 
 extern unsigned bitmap_first_set_bit (const_bitmap);
+extern unsigned bitmap_clear_first_set_bit (bitmap);
 extern unsigned bitmap_last_set_bit (const_bitmap);
 
 /* Compute bitmap hash (for purposes of hashing etc.)  */
Index: gcc/tree-ssa-propagate.c
===================================================================
--- gcc/tree-ssa-propagate.c	(revision 265185)
+++ gcc/tree-ssa-propagate.c	(working copy)
@@ -213,9 +213,6 @@ ssa_propagation_engine::simulate_stmt (g
   edge taken_edge = NULL;
   tree output_name = NULL_TREE;
 
-  /* Pull the stmt off the SSA edge worklist.  */
-  bitmap_clear_bit (ssa_edge_worklist, gimple_uid (stmt));
-
   /* Don't bother visiting statements that are already
      considered varying by the propagator.  */
   if (!prop_simulate_again_p (stmt))
@@ -322,7 +319,10 @@ ssa_propagation_engine::simulate_block (
   /* Always simulate PHI nodes, even if we have simulated this block
      before.  */
   for (gsi = gsi_start_phis (block); !gsi_end_p (gsi); gsi_next (&gsi))
-    simulate_stmt (gsi_stmt (gsi));
+    {
+      bitmap_clear_bit (ssa_edge_worklist, gimple_uid (gsi_stmt (gsi)));
+      simulate_stmt (gsi_stmt (gsi));
+    }
 
   /* If this is the first time we've simulated this block, then we
      must simulate each of its statements.  */
@@ -334,7 +334,10 @@ ssa_propagation_engine::simulate_block (
       edge_iterator ei;
 
       for (j = gsi_start_bb (block); !gsi_end_p (j); gsi_next (&j))
-	simulate_stmt (gsi_stmt (j));
+	{
+	  bitmap_clear_bit (ssa_edge_worklist, gimple_uid (gsi_stmt (j)));
+	  simulate_stmt (gsi_stmt (j));
+	}
 
       /* Note that we have simulated this block.  */
       block->flags |= BB_VISITED;
@@ -794,7 +797,8 @@ ssa_propagation_engine::ssa_propagate (v
 	      || next_block_order <= next_stmt_bb_order))
 	{
 	  curr_order = next_block_order;
-	  bitmap_clear_bit (cfg_blocks, next_block_order);
+	  int num = bitmap_clear_first_set_bit (cfg_blocks);
+	  gcc_checking_assert (num == next_block_order);
 	  basic_block bb
 	    = BASIC_BLOCK_FOR_FN (cfun, cfg_order_to_bb [next_block_order]);
 	  simulate_block (bb);
@@ -808,6 +812,8 @@ ssa_propagation_engine::ssa_propagate (v
 	      fprintf (dump_file, "\nSimulating statement: ");
 	      print_gimple_stmt (dump_file, next_stmt, 0, dump_flags);
 	    }
+	  int num = bitmap_clear_first_set_bit (ssa_edge_worklist);
+	  gcc_checking_assert (num == next_stmt_uid);
 	  simulate_stmt (next_stmt);
 	}
     }
Index: gcc/tree-ssa-sccvn.c
===================================================================
--- gcc/tree-ssa-sccvn.c	(revision 265185)
+++ gcc/tree-ssa-sccvn.c	(working copy)
@@ -6585,8 +6585,7 @@ do_rpo_vn (function *fn, edge entry, bit
       bitmap_set_bit (worklist, 0);
       while (!bitmap_empty_p (worklist))
 	{
-	  int idx = bitmap_first_set_bit (worklist);
-	  bitmap_clear_bit (worklist, idx);
+	  int idx = bitmap_clear_first_set_bit (worklist);
 	  basic_block bb = BASIC_BLOCK_FOR_FN (fn, rpo[idx]);
 	  gcc_assert ((bb->flags & BB_EXECUTABLE)
 		      && !rpo_state[idx].visited);
Index: gcc/tree-ssa-dce.c
===================================================================
--- gcc/tree-ssa-dce.c	(revision 265185)
+++ gcc/tree-ssa-dce.c	(working copy)
@@ -1698,8 +1698,7 @@ simple_dce_from_worklist (bitmap worklis
   while (! bitmap_empty_p (worklist))
     {
       /* Pop item.  */
-      unsigned i = bitmap_first_set_bit (worklist);
-      bitmap_clear_bit (worklist, i);
+      unsigned i = bitmap_clear_first_set_bit (worklist);
 
       tree def = ssa_name (i);
       /* Removed by somebody else or still in use.  */
Index: gcc/tree-cfgcleanup.c
===================================================================
--- gcc/tree-cfgcleanup.c	(revision 265185)
+++ gcc/tree-cfgcleanup.c	(working copy)
@@ -908,8 +908,7 @@ cleanup_tree_cfg_noloop (void)
   /* Now process the altered blocks, as long as any are available.  */
   while (!bitmap_empty_p (cfgcleanup_altered_bbs))
     {
-      unsigned i = bitmap_first_set_bit (cfgcleanup_altered_bbs);
-      bitmap_clear_bit (cfgcleanup_altered_bbs, i);
+      unsigned i = bitmap_clear_first_set_bit (cfgcleanup_altered_bbs);
       if (i < NUM_FIXED_BLOCKS)
 	continue;
 



More information about the Gcc-patches mailing list