[PATCH] D87865: PR47468: Fix findPHICopyInsertPoint, so that copies aren't incorrectly inserted after an INLINEASM_BR.

Nick Desaulniers via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 17 16:09:40 PDT 2020


nickdesaulniers added inline comments.


================
Comment at: llvm/lib/CodeGen/PHIEliminationUtils.cpp:43
+      DefsInMBB.insert(&RI);
   }
 
----------------
drop these extra parens?


================
Comment at: llvm/lib/CodeGen/PHIEliminationUtils.cpp:45
 
-  MachineBasicBlock::iterator InsertPoint;
-  if (DefUsesInMBB.empty()) {
-    // No defs.  Insert the copy at the start of the basic block.
-    InsertPoint = MBB->begin();
-  } else if (DefUsesInMBB.size() == 1) {
-    // Insert the copy immediately after the def/use.
-    InsertPoint = *DefUsesInMBB.begin();
-    ++InsertPoint;
-  } else {
-    // Insert the copy immediately after the last def/use.
-    InsertPoint = MBB->end();
-    while (!DefUsesInMBB.count(&*--InsertPoint)) {}
-    ++InsertPoint;
+  bool EHPadSuccessor = SuccMBB->isEHPad();
+
----------------
Hoist above L34 and then reuse `EHPadSuccessor`?


================
Comment at: llvm/lib/CodeGen/PHIEliminationUtils.cpp:52
+  for (auto I = MBB->rbegin(), E = MBB->rend(); I != E; ++I) {
+    if (DefsInMBB.count(&*I)) {
+      InsertPoint = std::next(I.getReverse());
----------------
`if (DefsInMBB.contains(&*I)) {`  (I know it's the same impl...and the previous version used that method)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87865



More information about the llvm-commits mailing list