[PATCH] D57123: [MergeSets] Add infrastructure to build merge sets based on Das and Ramakrishna's paper.

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added a comment.

Thanks for rebasing the patch.



================
Comment at: llvm/lib/Analysis/MergeSets.cpp:47
+
+void MergeSets::build(BasicBlock *Root) {
+  DT.updateDFSNumbers();
----------------
Some comments would be nice!


================
Comment at: llvm/lib/Analysis/MergeSets.cpp:109
+void MergeSets::build2(BasicBlock *Root) {
+  DT.updateDFSNumbers();
+
----------------
Is it possible that DFS numbers are already up-to-date?


================
Comment at: llvm/lib/Transforms/Utils/PromoteMemoryToRegister.cpp:660
+      // Make sure both approaches result in the same number of PHIs placed.
+      assert(NumAdded == IDFPHIBlocks.size() && "Number of blocks mismatched");
+
----------------
We can put the verification part somewhere else, possibly under a flag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57123