Allow "callbr" to return non-void values

Summary:
Terminators in LLVM aren't prohibited from returning values. This means that
the "callbr" instruction, which is used for "asm goto", can support "asm goto
with outputs."

This patch removes all restrictions against "callbr" returning values. The
heavy lifting is done by the code generator. The "INLINEASM_BR" instruction's
a terminator, and the code generator doesn't allow non-terminator instructions
after a terminator. In order to correctly model the feature, we need to copy
outputs from "INLINEASM_BR" into virtual registers. Of course, those copies
aren't terminators.

To get around this issue, we split the block containing the "INLINEASM_BR"
right before the "COPY" instructions. This results in two cheats:

  - Any physical registers defined by "INLINEASM_BR" need to be marked as
    live-in into the block with the "COPY" instructions. This violates an
    assumption that physical registers aren't marked as "live-in" until after
    register allocation. But it seems as if the live-in information only
    needs to be correct after register allocation. So we're able to get away
    with this.

  - The indirect branches from the "INLINEASM_BR" are moved to the "COPY"
    block. This is to satisfy PHI nodes.

I've been told that MLIR can support this handily, but until we're able to
use it, we'll have to stick with the above.

Reviewers: jyknight, nickdesaulniers, hfinkel, MaskRay, lattner

Reviewed By: nickdesaulniers, MaskRay, lattner

Subscribers: rriddle, qcolombet, jdoerfert, MatzeB, echristo, MaskRay, xbolva00, aaron.ballman, cfe-commits, JonChesterfield, hiraditya, llvm-commits, rnk, craig.topper

Tags: #llvm, #clang

Differential Revision: https://reviews.llvm.org/D69868

1 files changed, 32 insertions, 11 deletions

diff --git a/llvm/lib/CodeGen/MachineVerifier.cpp b/llvm/lib/CodeGen/MachineVerifier.cpp
index af3b8fd..69ecb45 100644
--- a/llvm/lib/CodeGen/MachineVerifier.cpp
+++ b/llvm/lib/CodeGen/MachineVerifier.cpp
@@ -622,6 +622,7 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
     // it is an entry block or landing pad.
     for (const auto &LI : MBB->liveins()) {
       if (isAllocatable(LI.PhysReg) && !MBB->isEHPad() &&
+          !MBB->isInlineAsmBrDefaultTarget() &&
           MBB->getIterator() != MBB->getParent()->begin()) {
         report("MBB has allocatable live-in, but isn't entry or landing-pad.", MBB);
         report_context(LI.PhysReg);
@@ -630,17 +631,30 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
   }
 
   // Count the number of landing pad successors.
-  SmallPtrSet<MachineBasicBlock*, 4> LandingPadSuccs;
-  for (MachineBasicBlock::const_succ_iterator I = MBB->succ_begin(),
-       E = MBB->succ_end(); I != E; ++I) {
-    if ((*I)->isEHPad())
-      LandingPadSuccs.insert(*I);
-    if (!FunctionBlocks.count(*I))
+  SmallPtrSet<const MachineBasicBlock*, 4> LandingPadSuccs;
+  for (const auto *succ : MBB->successors()) {
+    if (succ->isEHPad())
+      LandingPadSuccs.insert(succ);
+    if (!FunctionBlocks.count(succ))
       report("MBB has successor that isn't part of the function.", MBB);
-    if (!MBBInfoMap[*I].Preds.count(MBB)) {
+    if (!MBBInfoMap[succ].Preds.count(MBB)) {
       report("Inconsistent CFG", MBB);
       errs() << "MBB is not in the predecessor list of the successor "
-             << printMBBReference(*(*I)) << ".\n";
+             << printMBBReference(*succ) << ".\n";
+    }
+  }
+
+  // Count the number of INLINEASM_BR indirect target successors.
+  SmallPtrSet<const MachineBasicBlock*, 4> IndirectTargetSuccs;
+  for (const auto *succ : MBB->successors()) {
+    if (MBB->isInlineAsmBrIndirectTarget(succ))
+      IndirectTargetSuccs.insert(succ);
+    if (!FunctionBlocks.count(succ))
+      report("MBB has successor that isn't part of the function.", MBB);
+    if (!MBBInfoMap[succ].Preds.count(MBB)) {
+      report("Inconsistent CFG", MBB);
+      errs() << "MBB is not in the predecessor list of the successor "
+             << printMBBReference(*succ) << ".\n";
     }
   }
 
@@ -681,11 +695,15 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
         // It's possible that the block legitimately ends with a noreturn
         // call or an unreachable, in which case it won't actually fall
         // out the bottom of the function.
-      } else if (MBB->succ_size() == LandingPadSuccs.size()) {
+      } else if (MBB->succ_size() == LandingPadSuccs.size() ||
+                 MBB->succ_size() == IndirectTargetSuccs.size()) {
         // It's possible that the block legitimately ends with a noreturn
         // call or an unreachable, in which case it won't actually fall
         // out of the block.
-      } else if (MBB->succ_size() != 1+LandingPadSuccs.size()) {
+      } else if ((LandingPadSuccs.size() &&
+                  MBB->succ_size() != 1 + LandingPadSuccs.size()) ||
+                 (IndirectTargetSuccs.size() &&
+                  MBB->succ_size() != 1 + IndirectTargetSuccs.size())) {
         report("MBB exits via unconditional fall-through but doesn't have "
                "exactly one CFG successor!", MBB);
       } else if (!MBB->isSuccessor(&*MBBI)) {
@@ -707,7 +725,10 @@ MachineVerifier::visitMachineBasicBlockBefore(const MachineBasicBlock *MBB) {
       // landingpad, accept it as valid control flow.
       if (MBB->succ_size() != 1+LandingPadSuccs.size() &&
           (MBB->succ_size() != 1 || LandingPadSuccs.size() != 1 ||
-           *MBB->succ_begin() != *LandingPadSuccs.begin())) {
+           *MBB->succ_begin() != *LandingPadSuccs.begin()) &&
+          MBB->succ_size() != 1 + IndirectTargetSuccs.size() &&
+          (MBB->succ_size() != 1 || IndirectTargetSuccs.size() != 1 ||
+           *MBB->succ_begin() != *IndirectTargetSuccs.begin())) {
         report("MBB exits via unconditional branch but doesn't have "
                "exactly one CFG successor!", MBB);
       } else if (!MBB->isSuccessor(TBB)) {