[PATCH] D69868: Allow "callbr" to return non-void values

[Bill Wendling via Phabricator via llvm-commits]

void added inline comments.


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:1115
+  if (auto *bb = Succ->getBasicBlock())
+    if (auto *cbr = dyn_cast<CallBrInst>(getBasicBlock()->getTerminator()))
+      if (cbr->getDefaultDest() != bb)
----------------
rnk wrote:
> This code really should be looking at the machine IR to know if there is a callbr. I think INLINEASM_BR is target independent, so you really can just loop through the MachineInstrs looking for it, and then look for the MBB in the operand list. I wonder if we should have a flag on the MBB to indicate that it ends in an INLINEASM_BR, since those break the MIR invariant that terminators are grouped together at the end of the block.
The problem is that I need to between the indirect branches and the default one from the `callbr` instruction. It's way more cumbersome to do that with the `INLINEASM_BR` instruction. If you feel strongly about it I'll make the change, but the `INLINEASM_BR` instruction is already strongly tied to `callbr`. Do you expect it to change anytime in the future?


================
Comment at: llvm/lib/CodeGen/MachineBasicBlock.cpp:1116
+    if (auto *cbr = dyn_cast<CallBrInst>(getBasicBlock()->getTerminator()))
+      if (cbr->getDefaultDest() != bb)
+        for (unsigned i = 0, e = cbr->getNumIndirectDests(); i != e; ++i)
----------------
rnk wrote:
> Is it possible for a BB to be both an indirect successor and a fallthrough successor? I suppose that could be the case with the Linux macro that gets the current PC.
> 
> In any case, it's probably safe to remove this condition, and then we don't have to worry.
It is possible. I have a testcase for it in this patch. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69868