[llvm-commits] [Please review][PATCH] Implement the block_iterator of Region based on df_iterator.

[Hongbin Zheng]

Hi,

A few days ago in the polly-dev list,
Antoine<trouve.antoine at gmail.com> pointed out that the implementation
of block_iterator of the (single-entry single-exit) Region class is
hard to understand,
so I try to implement the block_iterator of Region based on
df_iterator, which is simpler.

The general idea is, by iterate the BB in DF order from the entry of
the region and marking the exit block of the single-entry single-exit
region as visited when we build the begin iterator, the iteration will
never visit the exit block and its successors, i.e. the blocks may be
visited by the DF iterator but outside the region.

best regards
ether

---
 include/llvm/Analysis/RegionInfo.h |   67 ++++++++++++++++++++---------------
 lib/Analysis/RegionInfo.cpp        |   16 --------
 lib/Analysis/RegionPrinter.cpp     |    2 +-
 3 files changed, 39 insertions(+), 46 deletions(-)

diff --git a/include/llvm/Analysis/RegionInfo.h
b/include/llvm/Analysis/RegionInfo.h
index eae94e7..310dcd6 100644
--- a/include/llvm/Analysis/RegionInfo.h
+++ b/include/llvm/Analysis/RegionInfo.h
@@ -500,50 +500,59 @@ public:
   /// Region. The iterator also iterates over BasicBlocks that are elements of
   /// a subregion of this Region. It is therefore called a flat iterator.
   //@{
-  template <typename RegionNodeItT>
+  template <bool IsConst>
   class block_iterator_wrapper
-    : public std::iterator<std::forward_iterator_tag, BasicBlock, ptrdiff_t> {
-    typedef std::iterator<std::forward_iterator_tag, BasicBlock, ptrdiff_t>
+    : public df_iterator<typename conditional<IsConst,
+                                              const BasicBlock,
+                                              BasicBlock>::type*> {
+    typedef df_iterator<typename conditional<IsConst,
+                                             const BasicBlock,
+                                             BasicBlock>::type*>
       super;
-
-    RegionNodeItT Iter;
-
   public:
-    typedef block_iterator_wrapper<RegionNodeItT> Self;
+    typedef block_iterator_wrapper<IsConst> Self;
     typedef typename super::pointer pointer;

-    block_iterator_wrapper(RegionNodeItT Iter) : Iter(Iter) {}
-
-    bool operator==(const Self &RHS) const { return Iter == RHS.Iter; }
-    bool operator!=(const Self &RHS) const { return Iter != RHS.Iter; }
-    pointer operator*() const {
-      return (*Iter)->template getNodeAs<BasicBlock>();
+    // Construct the begin iterator.
+    block_iterator_wrapper(pointer Entry, pointer Exit) :
super(df_begin(Entry))
+    {
+      // Mark the exit of the region as visited, so that the children of the
+      // exit and the exit itself, i.e. the block outside the region will never
+      // be visited.
+      super::Visited.insert(Exit);
     }

+    // Construct the end iterator.
+    block_iterator_wrapper() : super(df_end<pointer>(0)) {}
+
+    block_iterator_wrapper(super I) : super(I) {}
+
     Self& operator++() {
-      ++Iter;
+      super::operator++();
       return *this;
     }
     Self operator++(int) {
-      Self tmp = *this;
-      ++*this;
-      return tmp;
-    }
-
-    const Self &operator=(const Self &I) {
-      Iter = I.Iter;
-      return *this;
+      return super::operator++(0);
     }
   };
-  typedef block_iterator_wrapper<block_node_iterator> block_iterator;
-  typedef block_iterator_wrapper<const_block_node_iterator>
-    const_block_iterator;

-  block_iterator block_begin();
-  block_iterator block_end();
+  typedef block_iterator_wrapper<false> block_iterator;
+  typedef block_iterator_wrapper<true>  const_block_iterator;
+
+  block_iterator block_begin() const {
+   return block_iterator(getEntry(), getExit());
+  }

-  const_block_iterator block_begin() const;
-  const_block_iterator block_end() const;
+  block_iterator block_end() const {
+   return block_iterator();
+  }
+
+  const_block_iterator const_block_begin() const {
+    return const_block_iterator(getEntry(), getExit());
+  }
+  const_block_iterator const_block_end() const {
+    return const_block_iterator();
+  }
   //@}

   /// @name Element Iterators
diff --git a/lib/Analysis/RegionInfo.cpp b/lib/Analysis/RegionInfo.cpp
index 5f4458b..868f483 100644
--- a/lib/Analysis/RegionInfo.cpp
+++ b/lib/Analysis/RegionInfo.cpp
@@ -262,22 +262,6 @@ Region::const_block_node_iterator
Region::block_node_end() const {
   return GraphTraits<FlatIt<const Region*> >::nodes_end(this);
 }

-Region::block_iterator Region::block_begin() {
-  return block_node_begin();
-}
-
-Region::block_iterator Region::block_end() {
-  return block_node_end();
-}
-
-Region::const_block_iterator Region::block_begin() const {
-  return block_node_begin();
-}
-
-Region::const_block_iterator Region::block_end() const {
-  return block_node_end();
-}
-
 Region::element_iterator Region::element_begin() {
   return GraphTraits<Region*>::nodes_begin(this);
 }
diff --git a/lib/Analysis/RegionPrinter.cpp b/lib/Analysis/RegionPrinter.cpp
index 8b23cc7..6b14fa6 100644
--- a/lib/Analysis/RegionPrinter.cpp
+++ b/lib/Analysis/RegionPrinter.cpp
@@ -121,7 +121,7 @@ struct DOTGraphTraits<RegionInfo*> : public
DOTGraphTraits<RegionNode*> {

     RegionInfo *RI = R->getRegionInfo();

-    for (Region::const_block_iterator BI = R->block_begin(),
+    for (Region::block_iterator BI = R->block_begin(),
          BE = R->block_end(); BI != BE; ++BI)
       if (RI->getRegionFor(*BI) == R)
         O.indent(2 * (depth + 1)) << "Node"
-- 
1.7.5.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Implement-the-block_iterator-of-Region-based-on-df_i.patch
Type: text/x-diff
Size: 5035 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120801/0b1efa9c/attachment.patch>