diff --git a/include/llvm/CodeGen/Analysis.h b/include/llvm/CodeGen/Analysis.h index 2e4dc49a1e2..f20185c4499 100644 --- a/include/llvm/CodeGen/Analysis.h +++ b/include/llvm/CodeGen/Analysis.h @@ -105,11 +105,21 @@ ISD::CondCode getICmpCondCode(ICmpInst::Predicate Pred); /// This function only tests target-independent requirements. bool isInTailCallPosition(ImmutableCallSite CS, const TargetMachine &TM); +/// Test if given that the input instruction is in the tail call position, if +/// there is an attribute mismatch between the caller and the callee that will +/// inhibit tail call optimizations. +/// \p AllowDifferingSizes is an output parameter which, if forming a tail call +/// is permitted, determines whether it's permitted only if the size of the +/// caller's and callee's return types match exactly. +bool attributesPermitTailCall(const Function *F, const Instruction *I, + const ReturnInst *Ret, + const TargetLoweringBase &TLI, + bool *AllowDifferingSizes = nullptr); + /// Test if given that the input instruction is in the tail call position if the /// return type or any attributes of the function will inhibit tail call /// optimization. -bool returnTypeIsEligibleForTailCall(const Function *F, - const Instruction *I, +bool returnTypeIsEligibleForTailCall(const Function *F, const Instruction *I, const ReturnInst *Ret, const TargetLoweringBase &TLI); diff --git a/lib/CodeGen/Analysis.cpp b/lib/CodeGen/Analysis.cpp index ebbcaeb3057..6582d24da82 100644 --- a/lib/CodeGen/Analysis.cpp +++ b/lib/CodeGen/Analysis.cpp @@ -525,6 +525,47 @@ bool llvm::isInTailCallPosition(ImmutableCallSite CS, const TargetMachine &TM) { F, I, Ret, *TM.getSubtargetImpl(*F)->getTargetLowering()); } +bool llvm::attributesPermitTailCall(const Function *F, const Instruction *I, + const ReturnInst *Ret, + const TargetLoweringBase &TLI, + bool *AllowDifferingSizes) { + // ADS may be null, so don't write to it directly. + bool DummyADS; + bool &ADS = AllowDifferingSizes ? *AllowDifferingSizes : DummyADS; + ADS = true; + + AttrBuilder CallerAttrs(F->getAttributes(), + AttributeSet::ReturnIndex); + AttrBuilder CalleeAttrs(cast(I)->getAttributes(), + AttributeSet::ReturnIndex); + + // Noalias is completely benign as far as calling convention goes, it + // shouldn't affect whether the call is a tail call. + CallerAttrs = CallerAttrs.removeAttribute(Attribute::NoAlias); + CalleeAttrs = CalleeAttrs.removeAttribute(Attribute::NoAlias); + + if (CallerAttrs.contains(Attribute::ZExt)) { + if (!CalleeAttrs.contains(Attribute::ZExt)) + return false; + + ADS = false; + CallerAttrs.removeAttribute(Attribute::ZExt); + CalleeAttrs.removeAttribute(Attribute::ZExt); + } else if (CallerAttrs.contains(Attribute::SExt)) { + if (!CalleeAttrs.contains(Attribute::SExt)) + return false; + + ADS = false; + CallerAttrs.removeAttribute(Attribute::SExt); + CalleeAttrs.removeAttribute(Attribute::SExt); + } + + // If they're still different, there's some facet we don't understand + // (currently only "inreg", but in future who knows). It may be OK but the + // only safe option is to reject the tail call. + return CallerAttrs == CalleeAttrs; +} + bool llvm::returnTypeIsEligibleForTailCall(const Function *F, const Instruction *I, const ReturnInst *Ret, @@ -538,37 +579,8 @@ bool llvm::returnTypeIsEligibleForTailCall(const Function *F, if (isa(Ret->getOperand(0))) return true; // Make sure the attributes attached to each return are compatible. - AttrBuilder CallerAttrs(F->getAttributes(), - AttributeSet::ReturnIndex); - AttrBuilder CalleeAttrs(cast(I)->getAttributes(), - AttributeSet::ReturnIndex); - - // Noalias is completely benign as far as calling convention goes, it - // shouldn't affect whether the call is a tail call. - CallerAttrs = CallerAttrs.removeAttribute(Attribute::NoAlias); - CalleeAttrs = CalleeAttrs.removeAttribute(Attribute::NoAlias); - - bool AllowDifferingSizes = true; - if (CallerAttrs.contains(Attribute::ZExt)) { - if (!CalleeAttrs.contains(Attribute::ZExt)) - return false; - - AllowDifferingSizes = false; - CallerAttrs.removeAttribute(Attribute::ZExt); - CalleeAttrs.removeAttribute(Attribute::ZExt); - } else if (CallerAttrs.contains(Attribute::SExt)) { - if (!CalleeAttrs.contains(Attribute::SExt)) - return false; - - AllowDifferingSizes = false; - CallerAttrs.removeAttribute(Attribute::SExt); - CalleeAttrs.removeAttribute(Attribute::SExt); - } - - // If they're still different, there's some facet we don't understand - // (currently only "inreg", but in future who knows). It may be OK but the - // only safe option is to reject the tail call. - if (CallerAttrs != CalleeAttrs) + bool AllowDifferingSizes; + if (!attributesPermitTailCall(F, I, Ret, TLI, &AllowDifferingSizes)) return false; const Value *RetVal = Ret->getOperand(0), *CallVal = I; diff --git a/lib/CodeGen/CodeGenPrepare.cpp b/lib/CodeGen/CodeGenPrepare.cpp index 15600af4d39..3bdf60c172a 100644 --- a/lib/CodeGen/CodeGenPrepare.cpp +++ b/lib/CodeGen/CodeGenPrepare.cpp @@ -23,6 +23,7 @@ #include "llvm/Analysis/TargetTransformInfo.h" #include "llvm/Analysis/ValueTracking.h" #include "llvm/Analysis/MemoryBuiltins.h" +#include "llvm/CodeGen/Analysis.h" #include "llvm/IR/CallSite.h" #include "llvm/IR/Constants.h" #include "llvm/IR/DataLayout.h" @@ -1983,14 +1984,6 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB) { if (PN && PN->getParent() != BB) return false; - // It's not safe to eliminate the sign / zero extension of the return value. - // See llvm::isInTailCallPosition(). - const Function *F = BB->getParent(); - AttributeSet CallerAttrs = F->getAttributes(); - if (CallerAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::ZExt) || - CallerAttrs.hasAttribute(AttributeSet::ReturnIndex, Attribute::SExt)) - return false; - // Make sure there are no instructions between the PHI and return, or that the // return is the first instruction in the block. if (PN) { @@ -2010,13 +2003,15 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB) { /// Only dup the ReturnInst if the CallInst is likely to be emitted as a tail /// call. + const Function *F = BB->getParent(); SmallVector TailCalls; if (PN) { for (unsigned I = 0, E = PN->getNumIncomingValues(); I != E; ++I) { CallInst *CI = dyn_cast(PN->getIncomingValue(I)); // Make sure the phi value is indeed produced by the tail call. if (CI && CI->hasOneUse() && CI->getParent() == PN->getIncomingBlock(I) && - TLI->mayBeEmittedAsTailCall(CI)) + TLI->mayBeEmittedAsTailCall(CI) && + attributesPermitTailCall(F, CI, RetI, *TLI)) TailCalls.push_back(CI); } } else { @@ -2033,7 +2028,8 @@ bool CodeGenPrepare::dupRetToEnableTailCallOpts(BasicBlock *BB) { continue; CallInst *CI = dyn_cast(&*RI); - if (CI && CI->use_empty() && TLI->mayBeEmittedAsTailCall(CI)) + if (CI && CI->use_empty() && TLI->mayBeEmittedAsTailCall(CI) && + attributesPermitTailCall(F, CI, RetI, *TLI)) TailCalls.push_back(CI); } } diff --git a/test/CodeGen/X86/tailcall-cgp-dup.ll b/test/CodeGen/X86/tailcall-cgp-dup.ll index a51bc889924..26b43e7fe4d 100644 --- a/test/CodeGen/X86/tailcall-cgp-dup.ll +++ b/test/CodeGen/X86/tailcall-cgp-dup.ll @@ -85,3 +85,23 @@ someThingWithValue.exit: ; preds = %if.else.i, %if.then %retval.0.i = bitcast i8* %retval.0.in.i to %0* ret %0* %retval.0.i } + + +; Correctly handle zext returns. +declare zeroext i1 @foo_i1() + +; CHECK-LABEL: zext_i1 +; CHECK: jmp _foo_i1 +define zeroext i1 @zext_i1(i1 %k) { +entry: + br i1 %k, label %land.end, label %land.rhs + +land.rhs: ; preds = %entry + %call1 = tail call zeroext i1 @foo_i1() + br label %land.end + +land.end: ; preds = %entry, %land.rhs + %0 = phi i1 [ false, %entry ], [ %call1, %land.rhs ] + ret i1 %0 +} +