[PATCH] D50479: Expose CFG Update struct. Define GraphTraits to get children given a snapshot CFG.

[Tim Shen via Phabricator via llvm-commits]

timshen accepted this revision.
timshen added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/llvm/IR/CFGDiff.h:60
+  GraphDiff() {}
+  GraphDiff(ArrayRef<cfg::Update<NodePtr>> Updates, bool InverseGraph = false) {
+    SmallVector<cfg::Update<NodePtr>, 4> LegalizedUpdates;
----------------
kuhar wrote:
> It's not clear to me what InverseGraph means here.
I imagine that if InverseGraph, then To is a predecessor of From. If not InverseGraph, To is a successor of From. Maybe you want to document that?


================
Comment at: include/llvm/IR/CFGDiff.h:87
+
+  const iterator_range<typename SmallVectorImpl<NodePtr>::const_iterator>
+  getAddedChildren(const NodePtr BB, bool InverseEdge,
----------------
"const" is not necessary here. It doesn't prevent the caller from removing the const anyway.


================
Comment at: include/llvm/Support/CFGUpdate.h:72
+    if (InverseGraph)
+      std::swap(From, To); // Reverse edge for postdominators.
+
----------------
There is no need to swap?


================
Comment at: include/llvm/Support/CFGUpdate.h:95
+    const auto &U = AllUpdates[i];
+    if (!InverseGraph)
+      Operations[{U.getFrom(), U.getTo()}] = int(i);
----------------
If the previous swap is removed, I guess this will always be `Operations[{U.getFrom(), U.getTo()}] = int(i);`.


Repository:
  rL LLVM

https://reviews.llvm.org/D50479