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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: include/llvm/IR/CFGDiff.h:60
+  GraphDiff() {}
+  GraphDiff(ArrayRef<cfg::Update<NodePtr>> Updates, bool InverseGraph = false) {
+    SmallVector<cfg::Update<NodePtr>, 4> LegalizedUpdates;
----------------
timshen wrote:
> 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?
What confuses me is it's already possible to Pass Inverse<BB*> as the template arguments, so it's not clear if InverseGraph is for the graph, the updates, or if it means to swap insertions and deletions of edges.

It would be really helpful to add a comment about it above.


================
Comment at: include/llvm/Support/CFGUpdate.h:72
+    if (InverseGraph)
+      std::swap(From, To); // Reverse edge for postdominators.
+
----------------
timshen wrote:
> There is no need to swap?
DomTree and PostDomTree allow you to pass the same update vector to both, but since PostDomTree operates on inverse CFG, you have to swap somewhere to make it work.

I believe this was just copied from the domtree code.


Repository:
  rL LLVM

https://reviews.llvm.org/D50479