[PATCH] D25191: Modify df_iterator to support post-order actions

[Daniel Berlin via llvm-commits]

On Mon, Oct 3, 2016 at 1:43 PM, David Callahan <dcallahan at fb.com> wrote:

> david2050 added a comment.
>
> For illustration, this is what the client code looks like (abstracted a
> bit) for at least one back edge in a loop.
>

This seems a lot more complicated than just modifying insert :)

If you give it a "from" argument, you only need to do the following, i
think.

  std::pair<iterator, bool> insert(BasicBlock *From BasicBlock *BB) {
   // If insert succeeds, we've never seen this before
          if (!Status.insert(BB).second) {
             // We've seen this before, so this is a back or cross edge. If
it's a cross edge, we must have both visited and completed the other block
by now,
             // otherwise, it's somewhere in our ancestors, by definition.
             if (!Completed.count(From))
               Backedge.insert(From, BB).

        }
}
        void completed(BasicBlock *BB) { Completed[BB] = true; }


(IE there are two cases. In the case of the cross-edge, since we know we
visited it already, if it's a *cross* edge, it has to have been in some
other part of the tree that we finished, and thus completed should be true.
In the case of the backedge, it's visited but not completed.

This is a bit easier to see if think of dfsnumin/dfsnumout.
dfsnumin gets assigned when we visit the first time and call insert.
dfsnumout gets assigned when completed happens.
If, on insert, you find it inserted already, and "from" is a thing with
dfsnumin valid but dfsnumout not, it's your ancestor
If they are both valid, it must be a cross-edge (and you can assert that
{dfsnumin, dfsnumout} is not contained in {from.dfsnum, from.dfsnumout})

Staring for a second, this is the method gcc uses (it explicitly tracks the
dfsnumin and dfsnumout for each block)






> (Note this code is still order dependent in that the specific edges
> selected as back edges is order dependent for irreducible loops but is well
> defined otherwise).
>
>       // This stores state for the depth-first iterator. In addition
>       // to recording which nodes have been visited we also record whether
>       // a node is currently on the "stack" of active ancestors of the
> current
>       // node.
>       class DFState {
>         DenseMap<BasicBlock *, bool> Status;
>
>       public:
>         typedef typename DenseMap<BasicBlock *, bool>::iterator iterator;
>
>         DFState(unsigned N) { Status.reserve(N); }
>         std::pair<iterator, bool> insert(BasicBlock *BB) {
>           return Status.insert(std::make_pair(BB, true));
>         }
>         unsigned count(BasicBlock *BB) { return Status.count(BB); }
>         // Invoked after we have visited all children of a node.
>         void completed(BasicBlock *BB) { Status[BB] = false; }
>         // Return true if \p BB is currently on the active stack
>         // of ancestors.
>         bool onStack(BasicBlock *BB) {
>           auto Iter = Status.find(BB);
>           return Iter != Status.end() && Iter->second;
>         }
>       } State(F.size());
>
>       // Iterate over blocks in depth-first pre-order and
>       // treat all edges to a block already seen as loop back edges
>       // and mark the branch live it if there is a back edge.
>       typedef df_iterator<BasicBlock *, DFState, true> Iterator;
>       auto Iter = Iterator::begin(&F.getEntryBlock(), State);
>       auto End = Iterator::end(&F.getEntryBlock(), State);
>       for (; Iter != End; ++Iter) {
>         auto *BB = *Iter;
>
>         for (auto Succ : successors(BB))
>           if (State.onStack(Succ)) {
>             // back edge....
>           }
>       }
>   }
>
>
> https://reviews.llvm.org/D25191
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161003/a549897a/attachment.html>