Re: [patch] Normalize bitmap iteration.

[Diego Novillo]

On 2012-10-31 13:43 , Lawrence Crowl wrote:


Rename the EXECUTE_IF_SET_IN_SBITMAP macro to EXECUTE_IF_SET_IN_BITMAP.  Its
implementation is now identical to that in bitmap.h.  To prevent redefinition
errors, both definitions are now guarded by #ifndef.  An alternate strategy
is to simply include bitmap.h from sbitmap.h.  As this would increase build
time, I have elected to use the #ifndef version.  I do not have a strong
preference here.


Me neither.  This seems easy enough.


  static inline void
-sbitmap_iter_init (sbitmap_iterator *i, const_sbitmap bmp, unsigned int min)
+bmp_iter_set_init (sbitmap_iterator *i, const_sbitmap bmp,
+  unsigned int min, unsigned *bit_no ATTRIBUTE_UNUSED)


So, we'll be changing this again, right?  Once we introduce the various 
iterator types?



Index: gcc/bitmap.h
===
--- gcc/bitmap.h(revision 193006)
+++ gcc/bitmap.h(working copy)
@@ -682,10 +682,13 @@ bmp_iter_and_compl (bitmap_iterator *bi,
 should be treated as a read-only variable as it contains loop
 state.  */

+#ifndef EXECUTE_IF_SET_IN_BITMAP
+/* See sbitmap.h for the other definition of EXECUTE_IF_SET_IN_BITMAP.  */


Ah... if we could only overload macro defintions ;)


The patch is OK.  Thanks.


Diego.

Re: [patch] Normalize bitmap iteration.

[Lawrence Crowl]

On 11/1/12, Diego Novillo dnovi...@google.com wrote:
 On 2012-10-31 13:43 , Lawrence Crowl wrote:
  Rename the EXECUTE_IF_SET_IN_SBITMAP macro to
  EXECUTE_IF_SET_IN_BITMAP.  Its implementation is now identical to
  that in bitmap.h.  To prevent redefinition errors, both definitions
  are now guarded by #ifndef.  An alternate strategy is to simply
  include bitmap.h from sbitmap.h.  As this would increase build time,
  I have elected to use the #ifndef version.  I do not have a strong
  preference here.

 Me neither.  This seems easy enough.

static inline void
  -sbitmap_iter_init (sbitmap_iterator *i, const_sbitmap bmp, unsigned int
  min)
  +bmp_iter_set_init (sbitmap_iterator *i, const_sbitmap bmp,
  +  unsigned int min, unsigned *bit_no ATTRIBUTE_UNUSED)

 So, we'll be changing this again, right?  Once we introduce the various
 iterator types?

If you mean C++ style iterators, yes.  If you mean the 'computing'
bitmap iterators, no.

 Index: gcc/bitmap.h
 ===
 --- gcc/bitmap.h (revision 193006)
 +++ gcc/bitmap.h (working copy)
 @@ -682,10 +682,13 @@ bmp_iter_and_compl (bitmap_iterator *bi,
  should be treated as a read-only variable as it contains loop
  state.  */

 +#ifndef EXECUTE_IF_SET_IN_BITMAP
 +/* See sbitmap.h for the other definition of EXECUTE_IF_SET_IN_BITMAP.
 */

 Ah... if we could only overload macro defintions ;)

I had the same though.

 The patch is OK.  Thanks.

Okay, I'll merge and commit.

-- 
Lawrence Crowl

[patch] Normalize bitmap iteration.

[Lawrence Crowl]

This patch renames sbitmap iterators to unify them with the bitmap iterators.

Remove the unused EXECUTE_IF_SET_IN_SBITMAP_REV, which has an unconventional
interface.

Rename the sbitmap_iter_* functions to match bitmap's bmp_iter_* functions.
Add an additional parameter to the initialization and next functions to
match the interface in bmp_iter_*.  This extra parameter is mostly hidden
by the use of the EXECUTE_IF macros.

Rename the EXECUTE_IF_SET_IN_SBITMAP macro to EXECUTE_IF_SET_IN_BITMAP.  Its
implementation is now identical to that in bitmap.h.  To prevent redefinition
errors, both definitions are now guarded by #ifndef.  An alternate strategy
is to simply include bitmap.h from sbitmap.h.  As this would increase build
time, I have elected to use the #ifndef version.  I do not have a strong
preference here.

The sbitmap_iterator type is still distinctly named because it is often
declared in contexts where the bitmap type is not obvious.  There are less
than 40 uses of this type, so the burden to modify it when changing bitmap
types is not large.

Tested on x86-64, config-list.mk testing in progress.

Okay for trunk?


Index: gcc/ChangeLog

2012-10-31  Lawrence Crowl  cr...@google.com

* sbitmap.h (sbitmap_iter_init): Rename bmp_iter_set_init and add
unused parameter to match bitmap iterator.  Update callers.
(sbitmap_iter_cond): Rename bmp_iter_set.  Update callers.
(sbitmap_iter_next): Rename bmp_iter_next and add unused parameter to
match bitmap iterator.  Update callers.
(EXECUTE_IF_SET_IN_SBITMAP_REV): Remove unused.
(EXECUTE_IF_SET_IN_SBITMAP): Rename EXECUTE_IF_SET_IN_BITMAP and
adjust to be identical to the definition in bitmap.h.  Conditionalize
the definition based on not having been defined.  Update callers.
* bitmap.h (EXECUTE_IF_SET_IN_BITMAP): Conditionalize the definition
based on not having been defined.  (To match the above.)

Index: gcc/tree-into-ssa.c
===
--- gcc/tree-into-ssa.c (revision 193006)
+++ gcc/tree-into-ssa.c (working copy)
@@ -2672,12 +2672,12 @@ prepare_names_to_update (bool insert_phi
   /* First process names in NEW_SSA_NAMES.  Otherwise, uses of old
  names may be considered to be live-in on blocks that contain
  definitions for their replacements.  */
-  EXECUTE_IF_SET_IN_SBITMAP (new_ssa_names, 0, i, sbi)
+  EXECUTE_IF_SET_IN_BITMAP (new_ssa_names, 0, i, sbi)
 prepare_def_site_for (ssa_name (i), insert_phi_p);

   /* If an old name is in NAMES_TO_RELEASE, we cannot remove it from
  OLD_SSA_NAMES, but we have to ignore its definition site.  */
-  EXECUTE_IF_SET_IN_SBITMAP (old_ssa_names, 0, i, sbi)
+  EXECUTE_IF_SET_IN_BITMAP (old_ssa_names, 0, i, sbi)
 {
   if (names_to_release == NULL || !bitmap_bit_p (names_to_release, i))
prepare_def_site_for (ssa_name (i), insert_phi_p);
@@ -2737,7 +2737,7 @@ dump_update_ssa (FILE *file)
   fprintf (file, N_i - { O_1 ... O_j } means that N_i replaces 
 O_1, ..., O_j\n\n);

-  EXECUTE_IF_SET_IN_SBITMAP (new_ssa_names, 0, i, sbi)
+  EXECUTE_IF_SET_IN_BITMAP (new_ssa_names, 0, i, sbi)
dump_names_replaced_by (file, ssa_name (i));
 }

@@ -3241,7 +3241,7 @@ update_ssa (unsigned update_flags)
 for traversal.  */
  sbitmap tmp = sbitmap_alloc (SBITMAP_SIZE (old_ssa_names));
  bitmap_copy (tmp, old_ssa_names);
- EXECUTE_IF_SET_IN_SBITMAP (tmp, 0, i, sbi)
+ EXECUTE_IF_SET_IN_BITMAP (tmp, 0, i, sbi)
insert_updated_phi_nodes_for (ssa_name (i), dfs, blocks_to_update,
  update_flags);
  sbitmap_free (tmp);
@@ -3265,7 +3265,7 @@ update_ssa (unsigned update_flags)

   /* Reset the current definition for name and symbol before renaming
  the sub-graph.  */
-  EXECUTE_IF_SET_IN_SBITMAP (old_ssa_names, 0, i, sbi)
+  EXECUTE_IF_SET_IN_BITMAP (old_ssa_names, 0, i, sbi)
 get_ssa_name_ann (ssa_name (i))-info.current_def = NULL_TREE;

   FOR_EACH_VEC_ELT (tree, symbols_to_rename, i, sym)
Index: gcc/sbitmap.c
===
--- gcc/sbitmap.c   (revision 193006)
+++ gcc/sbitmap.c   (working copy)
@@ -656,7 +656,7 @@ bitmap_first_set_bit (const_sbitmap bmap
   unsigned int n = 0;
   sbitmap_iterator sbi;

-  EXECUTE_IF_SET_IN_SBITMAP (bmap, 0, n, sbi)
+  EXECUTE_IF_SET_IN_BITMAP (bmap, 0, n, sbi)
 return n;
   return -1;
 }
Index: gcc/sbitmap.h
===
--- gcc/sbitmap.h   (revision 193006)
+++ gcc/sbitmap.h   (working copy)
@@ -45,7 +45,7 @@ along with GCC; see the file COPYING3.
  * cardinality : sbitmap_popcount
  * choose_one  : bitmap_first_set_bit /
  bitmap_last_set_bit
- * forall  :