[PATCH] D29243: Cache reverse graph edges during dominator construction to avoidhaving to look them up later.

Daniel Berlin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Feb 5 12:20:09 PST 2017


dberlin marked an inline comment as done.
dberlin added inline comments.


================
Comment at: include/llvm/Support/GenericDomTree.h:250
     NodeT *Label;
-
+    SmallVector<NodeT *, 2> ReverseChildren;
     InfoRec() : DFSNum(0), Parent(0), Semi(0), Label(nullptr) {}
----------------
davide wrote:
> Just wondering, why 2?
commented


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:206
     WInfo.Semi = WInfo.Parent;
-    typedef GraphTraits<Inverse<NodeT> > InvTraits;
-    for (typename InvTraits::ChildIteratorType CI =
-         InvTraits::child_begin(W),
-         E = InvTraits::child_end(W); CI != E; ++CI) {
-      typename InvTraits::NodeRef N = *CI;
-      if (DT.Info.count(N)) {  // Only if this predecessor is reachable!
+    for (auto *N : WInfo.ReverseChildren) {
         unsigned SemiU = DT.Info[Eval<GraphT>(DT, N, i + 1)].Semi;
----------------
davide wrote:
> Nit: I prefer the explicit type when non-obvious from the right hand side, and I just recently realized LLVM coding convention suggests this too. Anyway, this is just splitting hair, so, up to you.
Making this an explicit type is a bit annoying. The real type is really GraphTraits<Inverse<NodeT> >::NodeRef

You would think it's just NodeT * (since that's what is in the SmallVector) ,but  using NodeT causes it to screw up the inference of Calculate everywhere.

So i'm going to leave it auto :)



https://reviews.llvm.org/D29243





More information about the llvm-commits mailing list