[PATCH] D78234: [BranchFolding] assert when removing INLINEASM_BR indirect targets

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/CodeGen/BranchFolding.cpp:168
+          // Iterating I.terminators() may be unreliable in this case.
+          // https://reviews.llvm.org/D78166#1984487
+          for (const MachineOperand &Op : I.operands())
----------------
It would be better to expand the comment, instead of incorporating the review by reference.


================
Comment at: llvm/lib/CodeGen/BranchFolding.cpp:171
+            if (Op.isBlockAddress() &&
+                Op.getBlockAddress()->getBasicBlock() == MBB->getBasicBlock())
+              assert(false &&
----------------
`Op.isBlockAddress()` check isn't sufficient to filter out all the relevant cases, I think.  It's possible to pass a blockaddress to an asm that isn't a valid destination according to the IR.


================
Comment at: llvm/lib/CodeGen/BranchFolding.cpp:173
+              assert(false &&
+                     "Removing MBB that's an indirect target of INLINEASM_BR");
+#endif
----------------
llvm_unreachable?

I'm a little concerned iterating over the entire function is going to be expensive, but we probably erase address-taken blocks pretty rarely.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78234