diff --git a/llvm/include/llvm/Transforms/IPO/Attributor.h b/llvm/include/llvm/Transforms/IPO/Attributor.h --- a/llvm/include/llvm/Transforms/IPO/Attributor.h +++ b/llvm/include/llvm/Transforms/IPO/Attributor.h @@ -591,6 +591,13 @@ return AG.getAnalysis(F); } + /// Return true if \p Arg is involved in a must-tail call, thus the argument + /// of the caller or callee. + bool isInvolvedInMustTailCall(const Argument &Arg) const { + return FunctionsCalledViaMustTail.count(Arg.getParent()) || + FunctionsWithMustTailCall.count(Arg.getParent()); + } + /// Return the analysis result from a pass \p AP for function \p F. template typename AP::Result *getAnalysisResultForFunction(const Function &F) { @@ -621,6 +628,12 @@ /// A map from functions to their instructions that may read or write memory. FuncRWInstsMapTy FuncRWInstsMap; + /// Functions called by a `musttail` call. + SmallPtrSet FunctionsCalledViaMustTail; + + /// Functions containing a `musttail` call. + SmallPtrSet FunctionsWithMustTailCall; + /// The datalayout used in the module. const DataLayout &DL; diff --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp --- a/llvm/lib/Transforms/IPO/Attributor.cpp +++ b/llvm/lib/Transforms/IPO/Attributor.cpp @@ -1328,7 +1328,11 @@ // TODO: This should be handled differently! this->AnchorVal = UniqueRVArg; this->KindOrArgNo = UniqueRVArg->getArgNo(); - Changed = IRAttribute::manifest(A); + // If the associated argument is involved in a must-tail call we give up + // because we would need to keep the argument alignments of caller and + // callee in-sync. Just does not seem worth the trouble right now. + if (!A.getInfoCache().isInvolvedInMustTailCall(*UniqueRVArg)) + Changed = IRAttribute::manifest(A); } else if (auto *RVC = dyn_cast(UniqueRV.getValue())) { // We can replace the returned value with the unique returned constant. Value &AnchorValue = getAnchorValue(); @@ -4062,10 +4066,20 @@ struct AAAlignArgument final : AAArgumentFromCallSiteArgumentsAndMustBeExecutedContext { - AAAlignArgument(const IRPosition &IRP) - : AAArgumentFromCallSiteArgumentsAndMustBeExecutedContext( - IRP) {} + using Base = + AAArgumentFromCallSiteArgumentsAndMustBeExecutedContext; + AAAlignArgument(const IRPosition &IRP) : Base(IRP) {} + + /// See AbstractAttribute::manifest(...). + ChangeStatus manifest(Attributor &A) override { + // If the associated argument is involved in a must-tail call we give up + // because we would need to keep the argument alignments of caller and + // callee in-sync. Just does not seem worth the trouble right now. + if (A.getInfoCache().isInvolvedInMustTailCall(*getAssociatedArgument())) + return ChangeStatus::UNCHANGED; + return Base::manifest(A); + } /// See AbstractAttribute::trackStatistics() void trackStatistics() const override { STATS_DECLTRACK_ARG_ATTR(aligned) } @@ -4076,6 +4090,12 @@ /// See AbstractAttribute::manifest(...). ChangeStatus manifest(Attributor &A) override { + // If the associated argument is involved in a must-tail call we give up + // because we would need to keep the argument alignments of caller and + // callee in-sync. Just does not seem worth the trouble right now. + if (Argument *Arg = getAssociatedArgument()) + if (A.getInfoCache().isInvolvedInMustTailCall(*Arg)) + return ChangeStatus::UNCHANGED; ChangeStatus Changed = AAAlignImpl::manifest(A); MaybeAlign InheritAlign = getAssociatedValue().getPointerAlignment(A.getDataLayout()); @@ -8308,11 +8328,12 @@ "New call site/base instruction type needs to be known in the " "Attributor."); break; - case Instruction::Load: - // The alignment of a pointer is interesting for loads. - case Instruction::Store: - // The alignment of a pointer is interesting for stores. case Instruction::Call: + if (cast(I).isMustTailCall()) { + InfoCache.FunctionsWithMustTailCall.insert(&F); + InfoCache.FunctionsCalledViaMustTail.insert( + cast(I).getCalledFunction()); + } case Instruction::CallBr: case Instruction::Invoke: case Instruction::CleanupRet: @@ -8322,6 +8343,10 @@ case Instruction::Br: case Instruction::Resume: case Instruction::Ret: + case Instruction::Load: + // The alignment of a pointer is interesting for loads. + case Instruction::Store: + // The alignment of a pointer is interesting for stores. IsInterestingOpcode = true; } if (IsInterestingOpcode) @@ -8356,6 +8381,15 @@ if (F.isDeclaration()) return; + // In non-module runs we need to look at the call sites of a function to + // determine if it is part of a must-tail call edge. This will influence what + // attributes we can derive. + if (!isModulePass() && !InfoCache.FunctionsCalledViaMustTail.count(&F)) + for (const Use &U : F.uses()) + if (ImmutableCallSite ICS = ImmutableCallSite(U.getUser())) + if (ICS.isCallee(&U) && ICS.isMustTailCall()) + InfoCache.FunctionsCalledViaMustTail.insert(&F); + IRPosition FPos = IRPosition::function(F); // Check for dead BasicBlocks in every function. diff --git a/llvm/test/Transforms/Attributor/align.ll b/llvm/test/Transforms/Attributor/align.ll --- a/llvm/test/Transforms/Attributor/align.ll +++ b/llvm/test/Transforms/Attributor/align.ll @@ -552,6 +552,23 @@ store i8 0, i8* %bc ret void } + +; Make sure we do not annotate the callee of a must-tail call with an alignment +; we cannot also put on the caller. +@cnd = external global i1 +define i32 @musttail_callee_1(i32* %p) { + %v = load i32, i32* %p, align 32 + ret i32 %v +} +define i32 @musttail_caller_1(i32* %p) { + %c = load i1, i1* @cnd + br i1 %c, label %mt, label %exit +mt: + %v = musttail call i32 @musttail_callee_1(i32* %p) + ret i32 %v +exit: + ret i32 0 +} ; UTC_ARGS: --disable attributes #0 = { nounwind uwtable noinline }