Skip to content

Commit

Permalink
[GlobalISel] When a tail call is emitted in a block, stop translating it
Browse files Browse the repository at this point in the history
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

llvm-svn: 371572
  • Loading branch information
Jessica Paquette committed Sep 10, 2019
1 parent d40ddb9 commit 469d42f
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 12 deletions.
4 changes: 4 additions & 0 deletions llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h
Expand Up @@ -518,6 +518,10 @@ class IRTranslator : public MachineFunctionPass {
// 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:
Expand Down
17 changes: 16 additions & 1 deletion llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down
11 changes: 0 additions & 11 deletions llvm/lib/Target/AArch64/AArch64CallLowering.cpp
Expand Up @@ -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");
Expand Down
30 changes: 30 additions & 0 deletions llvm/test/CodeGen/AArch64/GlobalISel/call-translator-tail-call.ll
Expand Up @@ -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
}

0 comments on commit 469d42f

Please sign in to comment.