[PATCH] D67974: [Dominators][AMDGPU] Don't use virtual exit node in findNearestCommonDominator

Michael Liao via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 24 12:22:47 PDT 2019


hliao added inline comments.


================
Comment at: llvm/lib/CodeGen/MachinePostDominators.cpp:59
+  assert(!Blocks.empty());
+  assert(llvm::all_of(Blocks, [](MachineBasicBlock *B) { return B; }) &&
+         "Invalid block found");
----------------
no need to (expensive) check every BB is not null. findNearestCommonDomniator asserts on both of them are non-null.


================
Comment at: llvm/lib/CodeGen/MachinePostDominators.cpp:67
+    // Stop when the root is reached.
+    if (DT->isVirtualRoot(DT->getNode(NCD)))
+      break;
----------------
the logic here is weird. if virtual root is the NCD, that means there's no real NCD. but, breaking here returns wrong NCD. need to return nullptr explicitly here 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67974





More information about the llvm-commits mailing list