]> granicus.if.org Git - llvm/commitdiff
[GlobalISel] When a tail call is emitted in a block, stop translating it
authorJessica Paquette <jpaquette@apple.com>
Tue, 10 Sep 2019 23:34:45 +0000 (23:34 +0000)
committerJessica Paquette <jpaquette@apple.com>
Tue, 10 Sep 2019 23:34:45 +0000 (23:34 +0000)
This fixes a crash in tail call translation caused by assume and lifetime_end
intrinsics.

It's possible to have instructions other than a return after a tail call which
will still have `Analysis::isInTailCallPosition` return true. (Namely,
lifetime_end and assume intrinsics.)

If we emit a tail call, we should stop translating instructions in the block.
Otherwise, we can end up emitting an extra return, or dead instructions in
general. This makes the verifier unhappy, and is generally unfortunate for
codegen.

This also removes the code from AArch64CallLowering that checks if we have a
tail call when lowering a return. This is covered by the new code now.

Also update call-translator-tail-call.ll to show that we now properly tail call
in the presence of lifetime_end and assume.

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371572 91177308-0d34-0410-b5e6-96231b3b80d8

include/llvm/CodeGen/GlobalISel/IRTranslator.h
lib/CodeGen/GlobalISel/IRTranslator.cpp
lib/Target/AArch64/AArch64CallLowering.cpp
test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll

index 2e405ba7ec0ccd238f43b80f296ebb99d0ae3ee6..bdb92aa4689d34d341cc099e37e742e93d4674f0 100644 (file)
@@ -518,6 +518,10 @@ private:
   // function has the optnone attribute.
   bool EnableOpts = false;
 
+  /// True when the block contains a tail call. This allows the IRTranslator to
+  /// stop translating such blocks early.
+  bool HasTailCall = false;
+
   /// Switch analysis and optimization.
   class GISelSwitchLowering : public SwitchCG::SwitchLowering {
   public:
index 0a06d8aa172108cffb7ab832d735fdbf04d9cbf8..66082df0108b037346a4f477f6b6eb8144967f79 100644 (file)
@@ -32,6 +32,7 @@
 #include "llvm/CodeGen/MachineRegisterInfo.h"
 #include "llvm/CodeGen/StackProtector.h"
 #include "llvm/CodeGen/TargetFrameLowering.h"
+#include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/CodeGen/TargetLowering.h"
 #include "llvm/CodeGen/TargetPassConfig.h"
 #include "llvm/CodeGen/TargetRegisterInfo.h"
@@ -1568,6 +1569,13 @@ bool IRTranslator::translateCallSite(const ImmutableCallSite &CS,
       CLI->lowerCall(MIRBuilder, CS, Res, Args, SwiftErrorVReg,
                      [&]() { return getOrCreateVReg(*CS.getCalledValue()); });
 
+  // Check if we just inserted a tail call.
+  if (Success) {
+    assert(!HasTailCall && "Can't tail call return twice from block?");
+    const TargetInstrInfo *TII = MF->getSubtarget().getInstrInfo();
+    HasTailCall = TII->isTailCall(*std::prev(MIRBuilder.getInsertPt()));
+  }
+
   return Success;
 }
 
@@ -2276,8 +2284,15 @@ bool IRTranslator::runOnMachineFunction(MachineFunction &CurMF) {
       // Set the insertion point of all the following translations to
       // the end of this basic block.
       CurBuilder->setMBB(MBB);
-
+      HasTailCall = false;
       for (const Instruction &Inst : *BB) {
+        // If we translated a tail call in the last step, then we know
+        // everything after the call is either a return, or something that is
+        // handled by the call itself. (E.g. a lifetime marker or assume
+        // intrinsic.) In this case, we should stop translating the block and
+        // move on.
+        if (HasTailCall)
+          break;
 #ifndef NDEBUG
         Verifier.setCurrentInst(&Inst);
 #endif // ifndef NDEBUG
index 64e1c84d98d61d4d030a9e27167393858772771d..77944186c45333193c1f83442bc5c37debeaac79 100644 (file)
@@ -233,17 +233,6 @@ bool AArch64CallLowering::lowerReturn(MachineIRBuilder &MIRBuilder,
                                       const Value *Val,
                                       ArrayRef<Register> VRegs,
                                       Register SwiftErrorVReg) const {
-
-  // Check if a tail call was lowered in this block. If so, we already handled
-  // the terminator.
-  MachineFunction &MF = MIRBuilder.getMF();
-  if (MF.getFrameInfo().hasTailCall()) {
-    MachineBasicBlock &MBB = MIRBuilder.getMBB();
-    auto FirstTerm = MBB.getFirstTerminator();
-    if (FirstTerm != MBB.end() && FirstTerm->isCall())
-      return true;
-  }
-
   auto MIB = MIRBuilder.buildInstrNoInsert(AArch64::RET_ReallyLR);
   assert(((Val && !VRegs.empty()) || (!Val && VRegs.empty())) &&
          "Return value without a vreg");
index d253c54c1bb8c0533ed2189adc1923f9f1e8661b..c9ace6f4de158e35741c86f399336888a34c1f84 100644 (file)
@@ -183,3 +183,33 @@ define void @test_mismatched_caller() {
   tail call fastcc void @fast_fn()
   ret void
 }
+
+; Verify that lifetime markers and llvm.assume don't impact tail calling.
+declare void @llvm.assume(i1)
+define void @test_assume() local_unnamed_addr {
+  ; COMMON-LABEL: name: test_assume
+  ; COMMON: bb.1.entry:
+  ; COMMON:   TCRETURNdi @nonvoid_ret, 0, csr_aarch64_aapcs, implicit $sp
+entry:
+  %x = tail call i32 @nonvoid_ret()
+  %y = icmp ne i32 %x, 0
+  tail call void @llvm.assume(i1 %y)
+  ret void
+}
+
+declare void @llvm.lifetime.start.p0i8(i64, i8* nocapture)
+declare void @llvm.lifetime.end.p0i8(i64, i8* nocapture)
+define void @test_lifetime() local_unnamed_addr {
+  ; COMMON-LABEL: name: test_lifetime
+  ; COMMON: bb.1.entry:
+  ; COMMON:   [[FRAME_INDEX:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.t
+  ; COMMON:   LIFETIME_START %stack.0.t
+  ; COMMON:   TCRETURNdi @nonvoid_ret, 0, csr_aarch64_aapcs, implicit $sp
+entry:
+  %t = alloca i8, align 1
+  call void @llvm.lifetime.start.p0i8(i64 1, i8* %t)
+  %x = tail call i32 @nonvoid_ret()
+  %y = icmp ne i32 %x, 0
+  tail call void @llvm.lifetime.end.p0i8(i64 1, i8* %t)
+  ret void
+}