[llvm] r371572 - [GlobalISel] When a tail call is emitted in a block, stop translating it

Jessica Paquette via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 10 16:34:45 PDT 2019


Author: paquette
Date: Tue Sep 10 16:34:45 2019
New Revision: 371572

URL: http://llvm.org/viewvc/llvm-project?rev=371572&view=rev
Log:
[GlobalISel] When a tail call is emitted in a block, stop translating it

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

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

Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h?rev=371572&r1=371571&r2=371572&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h Tue Sep 10 16:34:45 2019
@@ -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:

Modified: llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp?rev=371572&r1=371571&r2=371572&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp Tue Sep 10 16:34:45 2019
@@ -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(con
       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(
       // 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

Modified: llvm/trunk/lib/Target/AArch64/AArch64CallLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64CallLowering.cpp?rev=371572&r1=371571&r2=371572&view=diff
==============================================================================
--- llvm/trunk/lib/Target/AArch64/AArch64CallLowering.cpp (original)
+++ llvm/trunk/lib/Target/AArch64/AArch64CallLowering.cpp Tue Sep 10 16:34:45 2019
@@ -233,17 +233,6 @@ bool AArch64CallLowering::lowerReturn(Ma
                                       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");

Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll?rev=371572&r1=371571&r2=371572&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll Tue Sep 10 16:34:45 2019
@@ -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
+}




More information about the llvm-commits mailing list