[GlobalISel] Ensure that translateInvoke adds all successors for inlineasm

[paquette]

Summary

The existing code didn't add all necessary successors, which resulted in disjoint basic blocks. These would end up not being legalized which, in the best case, caused a fallback only in assert builds.

Here's an example:

https://godbolt.org/z/ndx15Enfj

We also end up getting weird codegen here as well.

Refactoring the code here allows us to correctly attach all successors. With this patch, the above example gives correct codegen at -O0 with and without asserts.

Also autogen the testcase to show that we add all the successors now.

[arsenm]

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll

51

Is there a test missing for the !canThrow case?

[paquette]

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll

51

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-no-unwind-inline-asm.ll should test that path.

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateInvoke(const User &U,
   // FIXME: support control flow guard targets.
   if (I.countOperandBundlesOfType(LLVMContext::OB_cfguardtarget))
     return false;
 
   // FIXME: support Windows exception handling.
   if (!isa<LandingPadInst>(EHPadBB->getFirstNonPHI()))
     return false;
 
-  bool LowerInlineAsm = false;
-  if (I.isInlineAsm()) {
-    const InlineAsm *IA = cast<InlineAsm>(I.getCalledOperand());
-    if (!IA->canThrow()) {
-      // Fast path without emitting EH_LABELs.
-
-      if (!translateInlineAsm(I, MIRBuilder))
-        return false;
-
-      MachineBasicBlock *InvokeMBB = &MIRBuilder.getMBB(),
-                        *ReturnMBB = &getMBB(*ReturnBB);
-
-      // Update successor info.
-      addSuccessorWithProb(InvokeMBB, ReturnMBB, BranchProbability::getOne());
-
-      MIRBuilder.buildBr(*ReturnMBB);
-      return true;
-    } else {
-      LowerInlineAsm = true;
-    }
-  }
+  bool LowerInlineAsm = I.isInlineAsm();
+  bool NeedEHLabel = true;
+  // If it can't throw then use a fast-path without emitting EH labels.
+  if (LowerInlineAsm)
+    NeedEHLabel = (cast<InlineAsm>(I.getCalledOperand()))->canThrow();
 
   // Emit the actual call, bracketed by EH_LABELs so that the MF knows about
   // the region covered by the try.
-  MCSymbol *BeginSymbol = Context.createTempSymbol();
+  MCSymbol *BeginSymbol = nullptr;
+  if (NeedEHLabel) {
+    BeginSymbol = Context.createTempSymbol();
     MIRBuilder.buildInstr(TargetOpcode::EH_LABEL).addSym(BeginSymbol);
+  }
 
   if (LowerInlineAsm) {
     if (!translateInlineAsm(I, MIRBuilder))
       return false;
   } else if (!translateCallBase(I, MIRBuilder))
     return false;
 
-  MCSymbol *EndSymbol = Context.createTempSymbol();
+  MCSymbol *EndSymbol = nullptr;
+  if (NeedEHLabel) {
+    EndSymbol = Context.createTempSymbol();
     MIRBuilder.buildInstr(TargetOpcode::EH_LABEL).addSym(EndSymbol);
+  }
 
   SmallVector<std::pair<MachineBasicBlock *, BranchProbability>, 1> UnwindDests;
   BranchProbabilityInfo *BPI = FuncInfo.BPI;
   MachineBasicBlock *InvokeMBB = &MIRBuilder.getMBB();
   BranchProbability EHPadBBProb =
       BPI ? BPI->getEdgeProbability(InvokeMBB->getBasicBlock(), EHPadBB)
           : BranchProbability::getZero();
 
   if (!findUnwindDestinations(EHPadBB, EHPadBBProb, UnwindDests))
     return false;
 
   MachineBasicBlock &EHPadMBB = getMBB(*EHPadBB),
                     &ReturnMBB = getMBB(*ReturnBB);
   // Update successor info.
   addSuccessorWithProb(InvokeMBB, &ReturnMBB);
   for (auto &UnwindDest : UnwindDests) {
     UnwindDest.first->setIsEHPad();
     addSuccessorWithProb(InvokeMBB, UnwindDest.first, UnwindDest.second);
   }
   InvokeMBB->normalizeSuccProbs();
 
+  if (NeedEHLabel) {
+    assert(BeginSymbol && "Expected a begin symbol!");
+    assert(EndSymbol && "Expected an end symbol!");
     MF->addInvoke(&EHPadMBB, BeginSymbol, EndSymbol);
+  }
+
   MIRBuilder.buildBr(ReturnMBB);
   return true;
 }
 
 bool IRTranslator::translateCallBr(const User &U,
                                    MachineIRBuilder &MIRBuilder) {
   // FIXME: Implement this.
   return false;

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-unwind-inline-asm.ll

+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
 ; RUN: llc -O0 -global-isel -stop-after=irtranslator < %s | FileCheck %s
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64-unknown-linux-gnu"
 
 @.str.2 = private unnamed_addr constant [7 x i8] c"Boom!\0A\00", align 1
 
 define dso_local void @trap() {
+  ; CHECK-LABEL: name: trap
+  ; CHECK: bb.1.entry:
 entry:
   unreachable
 }
 
 define dso_local void @test() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
+  ; CHECK-LABEL: name: test
+  ; CHECK: bb.1.entry:
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.3(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[GV:%[0-9]+]]:_(p0) = G_GLOBAL_VALUE @.str.2
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:_(p0) = COPY [[GV]](p0)
+  ; CHECK-NEXT:   EH_LABEL <mcsymbol >
+  ; CHECK-NEXT:   INLINEASM &"bl trap", 1 /* sideeffect attdialect */
+  ; CHECK-NEXT:   EH_LABEL <mcsymbol >
+  ; CHECK-NEXT:   G_BR %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2.invoke.cont:
+  ; CHECK-NEXT:   RET_ReallyLR
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3.lpad (landing-pad):
+  ; CHECK-NEXT:   liveins: $x0, $x1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   EH_LABEL <mcsymbol >
+  ; CHECK-NEXT:   [[DEF:%[0-9]+]]:_(s128) = G_IMPLICIT_DEF
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:_(p0) = COPY $x0
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:_(p0) = COPY $x1
+  ; CHECK-NEXT:   [[PTRTOINT:%[0-9]+]]:_(s32) = G_PTRTOINT [[COPY2]](p0)
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   $x0 = COPY [[COPY]](p0)
+  ; CHECK-NEXT:   BL @printf, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   $x0 = COPY [[COPY1]](p0)
+  ; CHECK-NEXT:   BL @_Unwind_Resume, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
 entry:
 
-; CHECK-LABEL: name: test
-; CHECK: body:
-; CHECK-NEXT: bb.1.entry
-; CHECK: EH_LABEL
-; CHECK-NEXT: INLINEASM
-; CHECK-NEXT: EH_LABEL
 
   invoke void asm sideeffect unwind "bl trap", ""()
           to label %invoke.cont unwind label %lpad
 
 invoke.cont:
   ret void
 
 lpad:
-; CHECK: bb.3.lpad
-; CHECK: EH_LABEL
 
   %0 = landingpad { i8*, i32 }
           cleanup
   call void (i8*, ...) @printf(i8* getelementptr inbounds ([7 x i8], [7 x i8]* @.str.2, i64 0, i64 0))
   resume { i8*, i32 } %0
 
 }
 
+define void @test2() #0 personality i32 (...)* @__gcc_personality_v0 {
+  ; CHECK-LABEL: name: test2
+  ; CHECK: bb.1 (%ir-block.0):
+  ; CHECK-NEXT:   successors: %bb.2(0x40000000), %bb.3(0x40000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[DEF:%[0-9]+]]:_(p0) = G_IMPLICIT_DEF
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gpr64common = COPY [[DEF]](p0)
+  ; CHECK-NEXT:   INLINEASM &"", 1 /* sideeffect attdialect */, 1572873 /* reguse:GPR64common */, [[COPY]]
+  ; CHECK-NEXT:   G_BR %bb.2
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2.a:
+  ; CHECK-NEXT:   RET_ReallyLR
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3.b (landing-pad):
+  ; CHECK-NEXT:   liveins: $x0, $x1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   EH_LABEL <mcsymbol >
+  ; CHECK-NEXT:   [[DEF1:%[0-9]+]]:_(s128) = G_IMPLICIT_DEF
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:_(p0) = COPY $x0
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:_(p0) = COPY $x1
+  ; CHECK-NEXT:   [[PTRTOINT:%[0-9]+]]:_(s32) = G_PTRTOINT [[COPY2]](p0)
+  ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
+  ; CHECK-NEXT:   $x0 = COPY [[COPY1]](p0)
+  ; CHECK-NEXT:   BL @_Unwind_Resume, csr_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0
+  ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
+  invoke void asm sideeffect "", "r"(i64* undef) to label %a unwind label %b
+a:
+  ret void
+b:
+  %landing_pad = landingpad { i8*, i32 } cleanup
+  resume { i8*, i32 } %landing_pad
+}
+
+declare i32 @__gcc_personality_v0(...)
 declare dso_local i32 @__gxx_personality_v0(...)
 
 declare dso_local void @printf(i8*, ...)