[PATCH] D77167: [GraphDiff] Extend GraphDiff to track a list of updates.

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar accepted this revision.
kuhar added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/include/llvm/IR/CFGDiff.h:86
+  // By default, it is assumed that, given a CFG and a set of updates, we wish
+  // to apply these updates. If UpdatedAreReverseApplied is set, the updates
+  // will be applied in reverse: deleted edges are considered re-added and
----------------
nit: to apply these updates forward/as-is/...?


================
Comment at: llvm/include/llvm/IR/CFGDiff.h:92
   // children.
   SmallVector<NodePtr, 1> Empty;
 
----------------
I don't think it makes any difference whatsoever, but maybe set the size to 0 here? There's a SmallVector specialization for that case.


================
Comment at: llvm/include/llvm/IR/CFGDiff.h:133
+
+  iterator_range<typename SmallVectorImpl<cfg::Update<NodePtr>>::const_iterator>
+  getLegalizedUpdates() const {
----------------
auto?


================
Comment at: llvm/include/llvm/IR/CFGDiff.h:138
+
+  unsigned getNoLegalizedUpdates() const { return LegalizedUpdates.size(); }
+
----------------
nit: `getNumLegalizedUpdates`


================
Comment at: llvm/include/llvm/Support/CFGUpdate.h:107
 
-  llvm::sort(Result,
+  if (!ReverseResultOrder)
+    llvm::
----------------
nit: this if-else spans 13 lines, consider adding braces


================
Comment at: llvm/include/llvm/Support/CFGUpdate.h:108
+  if (!ReverseResultOrder)
+    llvm::
+        sort(Result,
----------------
nit: this looks weird; did clang-format split it like this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77167