[PATCH] D63389: [IDF] Generalize IDFCalculator to be used with Clang's CFG

[Kristóf Umann via Phabricator via llvm-commits]

Szelethus updated this revision to Diff 205480.
Szelethus edited the summary of this revision.
Szelethus added a comment.

Reimplement D50675 <https://reviews.llvm.org/D50675>.

In D63389#1546512 <https://reviews.llvm.org/D63389#1546512>, @Szelethus wrote:

> In D63389#1546470 <https://reviews.llvm.org/D63389#1546470>, @asbirlea wrote:
>
> > We do very much need GraphDiff in the IDFCalculator when doing updates in MemorySSA. I don't know how I missed this, since it was the very reason it was added, but thanks a lot for catching it.
> >  I'll send a patch with the fix shortly.
>
>
> Glad its back! How does yet another class (`IDFGraphDiffCalculator`) sound like? With that, I could use template specializations instead of virtual functions.


That wasn't going to happen, since the `GraphDiff` object has to be used in the `getChildren` function.  I made `llvm::IDFCalculator::getChildren` virtual, is this okay?

I also have a local branch on which I used `std::function` (had to be used in order to pass a lambda with a non-empty capture list) and kept everything non-virtual, but I read somewhere that std::function is heavy-weight. Which solution is better, if any?

//(diffed to this revision)//

  diff --git a/llvm/include/llvm/Analysis/IteratedDominanceFrontier.h b/llvm/include/llvm/Analysis/IteratedDominanceFrontier.h
  index d3563f409fa..2ef6c9460bb 100644
  --- a/llvm/include/llvm/Analysis/IteratedDominanceFrontier.h
  +++ b/llvm/include/llvm/Analysis/IteratedDominanceFrontier.h
  @@ -27,7 +27,19 @@ public:
   
     IDFCalculator(DominatorTreeBase<BasicBlock, IsPostDom> &DT,
                   const GraphDiff<BasicBlock *, IsPostDom> *GD)
  -      : IDFCalculatorBase(DT), GD(GD) {
  +      : IDFCalculatorBase(
  +            DT,
  +            [GD](const NodeRef &BB) {
  +              using SnapShotBBPair =
  +                  std::pair<const GraphDiff<BasicBlock *, IsPostDom> *,
  +                            OrderedNodeTy>;
  +
  +              ChildrenTy Ret;
  +              for (auto Pair : children<SnapShotBBPair>({GD, BB}))
  +                Ret.emplace_back(Pair.second);
  +              return Ret;
  +            }),
  +        GD(GD) {
       assert(GD);
     }
   
  @@ -35,21 +47,6 @@ public:
     using ChildrenTy = typename IDFCalculatorBase::ChildrenTy;
     using OrderedNodeTy = typename IDFCalculatorBase::OrderedNodeTy;
   
  -  virtual ChildrenTy getChildren(const NodeRef &BB) override {
  -    if (!GD) {
  -      auto Children = children<OrderedNodeTy>(BB);
  -      return {Children.begin(), Children.end()};
  -    }
  -
  -    using SnapShotBBPair =
  -        std::pair<const GraphDiff<BasicBlock *, IsPostDom> *, OrderedNodeTy>;
  -
  -    ChildrenTy Ret;
  -    for (auto Pair : children<SnapShotBBPair>({GD, BB}))
  -      Ret.emplace_back(Pair.second);
  -    return Ret;
  -  }
  -
   private:
     const GraphDiff<BasicBlock *, IsPostDom> *GD = nullptr;
   };
  diff --git a/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h b/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
  index 15326a24955..e9dddb8c6d1 100644
  --- a/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
  +++ b/llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
  @@ -28,6 +28,7 @@
   #include "llvm/ADT/SmallVector.h"
   #include "llvm/IR/CFGDiff.h"
   #include "llvm/Support/GenericDomTree.h"
  +#include <functional>
   #include <queue>
   
   namespace llvm {
  @@ -46,9 +47,17 @@ public:
     using OrderedNodeTy =
         typename std::conditional<IsPostDom, Inverse<NodeTy *>, NodeTy *>::type;
   
  -  IDFCalculatorBase(DominatorTreeBase<NodeTy, IsPostDom> &DT) : DT(DT) {}
  +  using NodeRef = typename GraphTraits<OrderedNodeTy>::NodeRef;
  +  using ChildrenTy = SmallVector<NodeRef, 8>;
  +  using ChildrenGetterFn = ChildrenTy(const NodeRef &);
   
  -  virtual ~IDFCalculatorBase() = default;
  +  IDFCalculatorBase(DominatorTreeBase<NodeTy, IsPostDom> &DT,
  +                    std::function<ChildrenGetterFn> Fn =
  +                        [](const NodeRef &N) -> ChildrenTy {
  +                      auto Children = children<OrderedNodeTy>(N);
  +                      return {Children.begin(), Children.end()};
  +                    })
  +      : DT(DT), GetChildren(Fn) {}
   
     /// Give the IDF calculator the set of blocks in which the value is
     /// defined.  This is equivalent to the set of starting blocks it should be
  @@ -85,19 +94,12 @@ public:
     /// would be dead.
     void calculate(SmallVectorImpl<NodeTy *> &IDFBlocks);
   
  -  using NodeRef = typename GraphTraits<OrderedNodeTy>::NodeRef;
  -  using ChildrenTy = SmallVector<NodeRef, 8>;
  -
  -  virtual ChildrenTy getChildren(const NodeRef &N) {
  -    auto Children = children<OrderedNodeTy>(N);
  -    return {Children.begin(), Children.end()};
  -  }
  -
   private:
     DominatorTreeBase<NodeTy, IsPostDom> &DT;
     bool useLiveIn = false;
     const SmallPtrSetImpl<NodeTy *> *LiveInBlocks;
     const SmallPtrSetImpl<NodeTy *> *DefBlocks;
  +  std::function<ChildrenGetterFn> GetChildren;
   };
   
   //===----------------------------------------------------------------------===//
  @@ -170,7 +172,7 @@ void IDFCalculatorBase<NodeTy, IsPostDom>::calculate(
                 SuccNode, std::make_pair(SuccLevel, SuccNode->getDFSNumIn())));
         };
   
  -      for (auto Succ : getChildren(BB))
  +      for (auto Succ : GetChildren(BB))
           DoWork(Succ);
   
         for (auto DomChild : *Node) {


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63389/new/

https://reviews.llvm.org/D63389

Files:
  llvm/include/llvm/Analysis/IteratedDominanceFrontier.h
  llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
  llvm/lib/Analysis/CMakeLists.txt
  llvm/lib/Analysis/IteratedDominanceFrontier.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D63389.205480.patch
Type: text/x-patch
Size: 16748 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190619/f487ae02/attachment.bin>