diff mbox

C++ edge_iterator (was: Re: [SH] PR 53976 - Add RTL pass to eliminate clrt, sett insns)

Message ID 1386798867.2455.76.camel@yam-132-YW-E178-FTW
State New
Headers show

Commit Message

Oleg Endo Dec. 11, 2013, 9:54 p.m. UTC 
Same message, but now with the correct patch attached.  Sorry.

On Thu, 2013-11-21 at 00:04 +0100, Steven Bosscher wrote:
> Declaring the edge_iterator inside the for() is not a good argument
> against FOR_EACH_EDGE. Of course, brownie points are up for grabs for
> the brave soul daring enough to make edge iterators be proper C++
> iterators... ;-)

So, I gave it a try -- see the attached patch.
It allows edge iteration to look more like STL container iteration:

for (basic_block::edge_iterator ei = bb->pred_edges ().begin ();
     ei != bb->pred_edges ().end (); ++ei)
{
  basic_block pred_bb = (*ei)->src;
  ...
}

The idea is to first make class vec look more like an STL container.
This would also allow iterating it with std::for_each and use C++11
range based for loop (in a few years maybe).  It would also make it
easier to replace vec uses with STL containers if needed and vice versa.

Then the
typedef struct basic_block_def* basic_block;

is replaced with a wrapper class 'basic_block', which is just a simple
POD wrapper around a basic_block_def*.  There should be no penalties
compared to passing/storing raw pointers.  Because of the union with
constructor restriction of C++98 an additional wrapper class
'basic_block_in_union' is required, which doesn't have any constructors
defined.

Having 'basic_block' as a class allows putting typedefs for the edge
iterator types in there (initially I tried putting the typedefs into
struct basic_block_def, but gengtype would bail out).
It would also be possible to have a free standing definition / typedef
of edge_iterator, but it would conflict with the existing one and
require too many changes at once.  Moreover, the iterator type actually
depends on the container type, which is vec<edge, ...>, and the
container type is defined/selected by the basic_block class.

The following
  basic_block pred_bb = (*ei)->src;

can also be written as
  basic_block pred_bb = ei->src;

after converting the edge typedef to a wrapper of edge_def*.

The idea of the approach is to allow co-existence of the new
edge_iterator and the old and thus be able to gradually convert code.
The wrappers around raw pointers also helo encapsulating the underlying
memory management issues.  For example, it would be much easier to
replace garbage collected objects with intrusive reference counting.

Comments and feedback appreciated.

Cheers,
Oleg
diff mbox

Patch

Index: gcc/config/sh/sh_optimize_sett_clrt.cc
===================================================================
--- gcc/config/sh/sh_optimize_sett_clrt.cc	(revision 205866)
+++ gcc/config/sh/sh_optimize_sett_clrt.cc	(working copy)
@@ -390,10 +390,10 @@ 
     {
       prev_visited_bb.push_back (bb);
 
-      for (edge_iterator ei = ei_start (bb->preds); !ei_end_p (ei);
-	   ei_next (&ei))
+      for (basic_block::edge_iterator ei = bb->pred_edges ().begin ();
+	   ei != bb->pred_edges ().end (); ++ei)
 	{
-	  basic_block pred_bb = ei_edge (ei)->src;
+	  basic_block pred_bb = (*ei)->src;
 	  pred_bb_count += 1;
 	  find_last_ccreg_values (BB_END (pred_bb), pred_bb, values_out,
 				  prev_visited_bb);
Index: gcc/dumpfile.c
===================================================================
--- gcc/dumpfile.c	(revision 205866)
+++ gcc/dumpfile.c	(working copy)
@@ -25,6 +25,7 @@ 
 #include "tree.h"
 #include "gimple-pretty-print.h"
 #include "context.h"
+#include "basic-block2.h"
 
 /* If non-NULL, return one past-the-end of the matching SUBPART of
    the WHOLE string.  */
Index: gcc/tracer.c
===================================================================
--- gcc/tracer.c	(revision 205866)
+++ gcc/tracer.c	(working copy)
@@ -102,7 +102,7 @@ 
 
   /* A transaction is a single entry multiple exit region.  It must be
      duplicated in its entirety or not at all.  */
-  g = last_stmt (CONST_CAST_BB (bb));
+  g = last_stmt (basic_block (bb));
   if (g && gimple_code (g) == GIMPLE_TRANSACTION)
     return true;
 
Index: gcc/dominance.c
===================================================================
--- gcc/dominance.c	(revision 205866)
+++ gcc/dominance.c	(working copy)
@@ -503,6 +503,8 @@ 
   TBB v, w, k, par;
   basic_block en_block;
   edge_iterator ei, einext;
+  TBB k1;
+  basic_block b;
 
   if (reverse)
     en_block = EXIT_BLOCK_PTR_FOR_FN (cfun);
@@ -538,9 +540,6 @@ 
          semidominator.  */
       while (!ei_end_p (ei))
 	{
-	  TBB k1;
-	  basic_block b;
-
 	  e = ei_edge (ei);
 	  b = (reverse) ? e->dest : e->src;
 	  einext = ei;
Index: gcc/vec.h
===================================================================
--- gcc/vec.h	(revision 205866)
+++ gcc/vec.h	(working copy)
@@ -482,6 +482,15 @@ 
   void quick_grow (unsigned len);
   void quick_grow_cleared (unsigned len);
 
+  /* STL like iterator interface.  */
+  typedef T* iterator;
+  typedef const T* const_iterator;
+
+  iterator begin (void) { return &m_vecdata[0]; }
+  iterator end (void) { return &m_vecdata[m_vecpfx.m_num]; }
+  const_iterator begin (void) const { return &m_vecdata[0]; }
+  const_iterator end (void) const { &m_vecdata[m_vecpfx.m_num]; }
+
   /* vec class can access our internal data and functions.  */
   template <typename, typename, typename> friend struct vec;
 
Index: gcc/ifcvt.c
===================================================================
--- gcc/ifcvt.c	(revision 205866)
+++ gcc/ifcvt.c	(working copy)
@@ -65,7 +65,7 @@ 
 
 #define IFCVT_MULTIPLE_DUMPS 1
 
-#define NULL_BLOCK	((basic_block) NULL)
+#define NULL_BLOCK	(basic_block ((basic_block_def*) NULL))
 
 /* True if after combine pass.  */
 static bool ifcvt_after_combine;
Index: gcc/coretypes.h
===================================================================
--- gcc/coretypes.h	(revision 205866)
+++ gcc/coretypes.h	(working copy)
@@ -152,8 +152,8 @@ 
 typedef struct edge_def *edge;
 typedef const struct edge_def *const_edge;
 struct basic_block_def;
-typedef struct basic_block_def *basic_block;
-typedef const struct basic_block_def *const_basic_block;
+class basic_block;
+class const_basic_block;
 
 #define obstack_chunk_alloc	((void *(*) (long)) xmalloc)
 #define obstack_chunk_free	((void (*) (void *)) free)
Index: gcc/emit-rtl.c
===================================================================
--- gcc/emit-rtl.c	(revision 205866)
+++ gcc/emit-rtl.c	(working copy)
@@ -4446,7 +4446,7 @@ 
 emit_note_after (enum insn_note subtype, rtx after)
 {
   rtx note = make_note_raw (subtype);
-  basic_block bb = BARRIER_P (after) ? NULL : BLOCK_FOR_INSN (after);
+  basic_block bb = BARRIER_P (after) ? (basic_block_def*)NULL : (basic_block_def*)BLOCK_FOR_INSN (after);
   bool on_bb_boundary_p = (bb != NULL && BB_END (bb) == after);
 
   if (note_outside_basic_block_p (subtype, on_bb_boundary_p))
@@ -4462,7 +4462,7 @@ 
 emit_note_before (enum insn_note subtype, rtx before)
 {
   rtx note = make_note_raw (subtype);
-  basic_block bb = BARRIER_P (before) ? NULL : BLOCK_FOR_INSN (before);
+  basic_block bb = BARRIER_P (before) ? (basic_block_def*)NULL : (basic_block_def*)BLOCK_FOR_INSN (before);
   bool on_bb_boundary_p = (bb != NULL && BB_HEAD (bb) == before);
 
   if (note_outside_basic_block_p (subtype, on_bb_boundary_p))
Index: gcc/tree-inline.h
===================================================================
--- gcc/tree-inline.h	(revision 205866)
+++ gcc/tree-inline.h	(working copy)
@@ -117,7 +117,7 @@ 
   struct pointer_set_t *statements_to_fold;
 
   /* Entry basic block to currently copied body.  */
-  basic_block entry_bb;
+  basic_block_def* entry_bb;
 
   /* For partial function versioning, bitmap of bbs to be copied,
      otherwise NULL.  */
Index: gcc/df.h
===================================================================
--- gcc/df.h	(revision 205866)
+++ gcc/df.h	(working copy)
@@ -392,7 +392,7 @@ 
 
   /* Artificial refs do not have an insn, so to get the basic block,
      it must be explicitly here.  */
-  basic_block bb;
+  basic_block_in_union bb;
 };
 
 
Index: gcc/basic-block2.h
===================================================================
--- gcc/basic-block2.h	(revision 0)
+++ gcc/basic-block2.h	(revision 0)
@@ -0,0 +1,81 @@ 
+#ifndef GCC_BASIC_BLOCK2_H
+#define GCC_BASIC_BLOCK2_H
+
+struct basic_block_def;
+
+class basic_block_in_union
+{
+public:
+  typedef vec<edge, va_gc>::iterator edge_iterator;
+  typedef vec<edge, va_gc>::const_iterator const_edge_iterator;
+
+  basic_block_in_union& operator = (basic_block_def* bb) { m_bb = bb; return *this; }
+
+  basic_block_def* operator -> (void) const { return m_bb; }
+
+  bool operator == (const basic_block_in_union& rhs) const { return m_bb == rhs.m_bb; }
+  bool operator != (const basic_block_in_union& rhs) const { return m_bb != rhs.m_bb; }
+
+  operator basic_block_def* (void) const { return m_bb; }
+
+  basic_block_def* get (void) const { return m_bb; }
+
+protected:
+  basic_block_def* m_bb;
+};
+
+class const_basic_block_in_union
+{
+public:
+  typedef vec<edge, va_gc>::const_iterator const_edge_iterator;
+
+  const_basic_block_in_union& operator = (const basic_block_def* bb) { m_bb = bb; return *this; }
+  const_basic_block_in_union& operator = (basic_block_def* bb) { m_bb = bb; return *this; }
+
+  const basic_block_def* operator -> (void) const { return m_bb; }
+
+  bool operator == (const basic_block_in_union& rhs) const { return m_bb == rhs.get (); }
+  bool operator != (const basic_block_in_union& rhs) const { return m_bb != rhs.get (); }
+
+  operator const basic_block_def* (void) const { return m_bb; }
+
+  const basic_block_def* get (void) const { return m_bb; }
+
+protected:
+  const basic_block_def* m_bb;
+};
+
+class const_basic_block;
+
+class basic_block : public basic_block_in_union
+{
+public:
+  basic_block (void) { }
+  basic_block (const basic_block& rhs) { m_bb = rhs.get (); }
+  basic_block (const basic_block_in_union& rhs) { m_bb = rhs.get (); }
+  basic_block (basic_block_def* bb) { m_bb = bb; }
+
+  explicit basic_block (void* p) { m_bb = (basic_block_def*)p; }
+  explicit basic_block (const const_basic_block& rhs);
+};
+
+class const_basic_block : public const_basic_block_in_union
+{
+public:
+  const_basic_block (void) { }
+  const_basic_block (const const_basic_block& rhs) { m_bb = rhs.get (); }
+  const_basic_block (const basic_block& rhs) { m_bb = rhs.get (); }
+  const_basic_block (const basic_block_in_union& rhs) { m_bb = rhs.get (); }
+  const_basic_block (const const_basic_block_in_union& rhs) { m_bb = rhs.get (); }
+  const_basic_block (basic_block_def* bb) { m_bb = bb; }
+  const_basic_block (const basic_block_def* bb) { m_bb = bb; }
+
+  explicit const_basic_block (const void* p) { m_bb = (const basic_block_def*)p; }
+};
+
+inline basic_block::basic_block (const const_basic_block& rhs) { m_bb = const_cast<basic_block_def*> (rhs.get ()); }
+
+
+
+
+#endif // GCC_BASIC_BLOCK2_H
Index: gcc/basic-block.h
===================================================================
--- gcc/basic-block.h	(revision 205866)
+++ gcc/basic-block.h	(working copy)
@@ -23,6 +23,7 @@ 
 #include "predict.h"
 #include "vec.h"
 #include "function.h"
+#include "basic-block2.h"
 
 /* Use gcov_type to hold basic block counters.  Should be at least
    64bit.  Although a counter cannot be negative, we use a signed
@@ -169,6 +170,12 @@ 
   vec<edge, va_gc> *preds;
   vec<edge, va_gc> *succs;
 
+  vec<edge, va_gc>& pred_edges (void) { return *preds; }
+  const vec<edge, va_gc>& pred_edges (void) const { return *preds; }
+
+  vec<edge, va_gc>& succ_edges (void) { return *succs; }
+  const vec<edge, va_gc>& succ_edges (void) const { return *succs; }
+
   /* Auxiliary info specific to a pass.  */
   PTR GTY ((skip (""))) aux;
 
Index: gcc/cfgloop.h
===================================================================
--- gcc/cfgloop.h	(revision 205866)
+++ gcc/cfgloop.h	(working copy)
@@ -24,6 +24,7 @@ 
 #include "bitmap.h"
 #include "sbitmap.h"
 #include "function.h"
+#include "basic-block2.h"
 
 /* Structure to hold decision about unrolling/peeling.  */
 enum lpt_dec
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c	(revision 205866)
+++ gcc/tree-cfg.c	(working copy)
@@ -7356,7 +7356,7 @@ 
 static bool
 gimple_block_ends_with_condjump_p (const_basic_block bb)
 {
-  gimple stmt = last_stmt (CONST_CAST_BB (bb));
+  gimple stmt = last_stmt (basic_block (bb));
   return (stmt && gimple_code (stmt) == GIMPLE_COND);
 }
 
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 205866)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -2033,7 +2033,7 @@ 
 
   bb->aux = stridx_to_strinfo;
   if (vec_safe_length (stridx_to_strinfo) && !strinfo_shared ())
-    (*stridx_to_strinfo)[0] = (strinfo) bb;
+    (*stridx_to_strinfo)[0] = (strinfo) bb.get ();
 }
 
 /* Callback for walk_dominator_tree.  Free strinfo vector if it is
@@ -2046,7 +2046,7 @@ 
     {
       stridx_to_strinfo = ((vec<strinfo, va_heap, vl_embed> *) bb->aux);
       if (vec_safe_length (stridx_to_strinfo)
-	  && (*stridx_to_strinfo)[0] == (strinfo) bb)
+	  && (*stridx_to_strinfo)[0] == (strinfo) bb.get ())
 	{
 	  unsigned int i;
 	  strinfo si;
Index: gcc/rtl.h
===================================================================
--- gcc/rtl.h	(revision 205866)
+++ gcc/rtl.h	(working copy)
@@ -29,6 +29,7 @@ 
 #include "alias.h"
 #include "hashtab.h"
 #include "flags.h"
+#include "basic-block2.h"
 
 /* Value used by some passes to "recognize" noop moves as valid
  instructions.  */
@@ -193,7 +194,7 @@ 
   addr_diff_vec_flags rt_addr_diff_vec_flags;
   struct cselib_val *rt_cselib;
   tree rt_tree;
-  basic_block rt_bb;
+  basic_block_in_union rt_bb;
   mem_attrs *rt_mem;
   reg_attrs *rt_reg;
   struct constant_descriptor_rtx *rt_constant;