diff --git a/clang/test/CodeGenOpenCL/as_type.cl b/clang/test/CodeGenOpenCL/as_type.cl --- a/clang/test/CodeGenOpenCL/as_type.cl +++ b/clang/test/CodeGenOpenCL/as_type.cl @@ -81,14 +81,14 @@ return __builtin_astype(x, global int*); } -//CHECK: define spir_func i32 @ptr_to_int(i32* %[[x:.*]]) +//CHECK: define spir_func i32 @ptr_to_int(i32* readnone %[[x:.*]]) //CHECK: %[[cast:.*]] = ptrtoint i32* %[[x]] to i32 //CHECK: ret i32 %[[cast]] int ptr_to_int(int *x) { return __builtin_astype(x, int); } -//CHECK: define spir_func <3 x i8> @ptr_to_char3(i32* %[[x:.*]]) +//CHECK: define spir_func <3 x i8> @ptr_to_char3(i32* readnone %[[x:.*]]) //CHECK: %[[cast1:.*]] = ptrtoint i32* %[[x]] to i32 //CHECK: %[[cast2:.*]] = bitcast i32 %[[cast1]] to <4 x i8> //CHECK: %[[astype:.*]] = shufflevector <4 x i8> %[[cast2]], <4 x i8> undef, <3 x i32> 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 @@ -22,10 +22,16 @@ struct AbstractAttribute; struct Attributor; +class AAResults; class Function; class Module; +namespace { +template using GetterTy = std::function; +using AAGetterTy = GetterTy; +} // anonymous namespace + /// Simple enum class that forces the status to be spelled out explicitly. /// ///{ @@ -40,6 +46,7 @@ /// The fixpoint analysis framework that drives the attribute deduction. struct Attributor { + Attributor(AAGetterTy &AARGetter) : AARGetter(AARGetter) {} ~Attributor() { DeleteContainerPointers(AllAbstractAttributes); } /// Run the fixpoint analyses on the call graph SCC \p SCC. @@ -72,6 +79,9 @@ return FunctionReadOrWriteInstsMap[&F]; } + /// Return the alias analysis results for \p F. + AAResults &getAAR(Function &F) { return AARGetter(F); } + private: /// Introduce a new abstract attribute into the fixpoint analysis. template AAType ®isterAA(AAType &AA, int ArgNo = -1); @@ -104,6 +114,9 @@ using KindToAbstractAttributeMap = DenseMap; DenseMap, KindToAbstractAttributeMap> AAMap; ///} + + /// Closure to get alias analysis results for a function. + AAGetterTy &AARGetter; }; /// ---------------------------------------------------------------------------- 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 @@ -18,6 +18,9 @@ #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/Statistic.h" +#include "llvm/Analysis/AliasAnalysis.h" +#include "llvm/Analysis/AssumptionCache.h" +#include "llvm/Analysis/BasicAliasAnalysis.h" #include "llvm/Analysis/CallGraph.h" #include "llvm/Analysis/CallGraphSCCPass.h" #include "llvm/Analysis/CaptureTracking.h" @@ -66,6 +69,10 @@ STATISTIC(NumFnArgumentWriteOnly, "Number of function arguments marked write-only"); +STATISTIC(NumFnReadNone, "Number of functions marked read-none"); +STATISTIC(NumFnReadOnly, "Number of functions marked read-only"); +STATISTIC(NumFnWriteOnly, "Number of functions marked write-only"); + // TODO: Determine a good default value. // // In the LLVM-TS and SPEC2006, 32 seems to not induce compile time overheads @@ -203,13 +210,16 @@ NumFnNoRecurse++; return; case Attribute::ReadNone: - NumFnArgumentReadNone++; + MP == AbstractAttribute::MP_ARGUMENT ? NumFnArgumentReadNone++ + : NumFnReadNone++; return; case Attribute::ReadOnly: - NumFnArgumentReadOnly++; + MP == AbstractAttribute::MP_ARGUMENT ? NumFnArgumentReadOnly++ + : NumFnReadOnly++; return; case Attribute::WriteOnly: - NumFnArgumentWriteOnly++; + MP == AbstractAttribute::MP_ARGUMENT ? NumFnArgumentWriteOnly++ + : NumFnWriteOnly++; return; default: return; @@ -1346,8 +1356,8 @@ /// as defined by the encoding bits. struct AAMemoryBehaviorImpl : public AAMemoryBehavior, IntegerState { - AAMemoryBehaviorImpl(Value &V) - : AAMemoryBehavior(V), IntegerState(BEST_STATE) { + AAMemoryBehaviorImpl(Value &V, AAResults &AAR) + : AAMemoryBehavior(V), IntegerState(BEST_STATE), AAR(AAR) { assert(getAssumed() == BEST_STATE && "Expected optimistic initialization!"); } @@ -1402,7 +1412,7 @@ /// Return the memory behavior information of \p V encoded in the IR. static void getKnownStateFromValue(const Value &V, IntegerState &State, - bool ValueOnly); + bool ValueOnly, AAResults *AAR = nullptr); /// See AbstractAttribute::getDeducedAttributes(Attributor &A). virtual void @@ -1413,12 +1423,16 @@ AbstractState &getState() override { return *this; } const AbstractState &getState() const override { return *this; } ///} + +protected: + /// The alias analysis results for the associated function. + AAResults &AAR; }; void AAMemoryBehaviorImpl::getKnownStateFromValue(const Value &V, IntegerState &State, - bool ValueOnly) { - + bool ValueOnly, + AAResults *AAR) { if (const Argument *Arg = dyn_cast(&V)) { if (Arg->hasAttribute(Attribute::ReadNone)) State.addKnownBits(NO_ACCESSES); @@ -1427,14 +1441,22 @@ else if (Arg->hasAttribute(Attribute::WriteOnly)) State.addKnownBits(NO_READS); if (!ValueOnly) - getKnownStateFromValue(*Arg->getParent(), State, ValueOnly); + getKnownStateFromValue(*Arg->getParent(), State, ValueOnly, AAR); } else if (const Function *Fn = dyn_cast(&V)) { - if (Fn->hasFnAttribute(Attribute::ReadNone)) - State.addKnownBits(NO_ACCESSES); - else if (Fn->hasFnAttribute(Attribute::ReadOnly)) - State.addKnownBits(NO_WRITES); - else if (Fn->hasFnAttribute(Attribute::WriteOnly)) - State.addKnownBits(NO_READS); + if (AAR) { + auto ModRef = AAR->getModRefBehavior(Fn); + if (AAResults::onlyReadsMemory(ModRef)) + State.addKnownBits(NO_WRITES); + if (AAResults::doesNotReadMemory(ModRef)) + State.addKnownBits(NO_READS); + } else { + if (Fn->hasFnAttribute(Attribute::ReadNone)) + State.addKnownBits(NO_ACCESSES); + else if (Fn->hasFnAttribute(Attribute::ReadOnly)) + State.addKnownBits(NO_WRITES); + else if (Fn->hasFnAttribute(Attribute::WriteOnly)) + State.addKnownBits(NO_READS); + } } } @@ -1442,20 +1464,102 @@ SmallVectorImpl &Attrs) const { LLVMContext &Ctx = getAnchoredValue().getContext(); assert(Attrs.size() == 0); - if (isAssumedReadNone()) + if (isAssumedReadNone()) { Attrs.push_back(Attribute::get(Ctx, Attribute::ReadNone)); - else if (isAssumedReadOnly()) + return; + } + + if (isAssumedReadOnly()) Attrs.push_back(Attribute::get(Ctx, Attribute::ReadOnly)); else if (isAssumedWriteOnly()) Attrs.push_back(Attribute::get(Ctx, Attribute::WriteOnly)); assert(Attrs.size() <= 1); } +/// An AA to represent the memory behavior function attributes. +struct AAMemoryBehaviorFunction final : public AAMemoryBehaviorImpl { + + /// See AAMemoryBehaviorImpl::AAMemoryBehaviorImpl(...). + AAMemoryBehaviorFunction(Function &F, AAResults &AAR) + : AAMemoryBehaviorImpl(F, AAR) {} + + /// See AbstractAttribute::initialize(...). + void initialize(Attributor &A) override { + Function &F = cast(getAnchoredValue()); + getKnownStateFromValue(F, *this, /* ValueOnly */ false, &AAR); + } + + /// See AbstractAttribute::updateImpl(Attributor &A). + virtual ChangeStatus updateImpl(Attributor &A) override; + + /// See AbstractAttribute::manifest(Attributor &A). + virtual ChangeStatus manifest(Attributor &A) override; + + /// See AbstractAttribute::getManifestPosition(). + virtual ManifestPosition getManifestPosition() const override { + return MP_FUNCTION; + } +}; + +ChangeStatus AAMemoryBehaviorFunction::updateImpl(Attributor &A) { + + // The current assumed state used to determine a change. + auto AssumedState = getAssumed(); + + Function &F = cast(getAnchoredValue()); + auto &ReadOrWriteInsts = A.getReadOrWriteInstsForFunction(F); + + for (const Instruction *I : ReadOrWriteInsts) { + assert(I->mayReadOrWriteMemory()); + + // If the instruction has an own memory behavior state, use it to restrict + // the local state. No further analysis is required as the other memory + // state is as optimistic as it gets. + auto *MemBehaviorAA = A.getAAFor(*this, *I); + if (MemBehaviorAA && MemBehaviorAA->getState().isValidState()) { + intersectAssumedBits(MemBehaviorAA->getAssumed()); + continue; + } + + // Remove access kind modifiers if necessary. + if (I->mayReadFromMemory()) + removeAssumedBits(NO_READS); + if (I->mayWriteToMemory()) + removeAssumedBits(NO_WRITES); + } + + return (AssumedState != getAssumed()) ? ChangeStatus::CHANGED + : ChangeStatus::UNCHANGED; +} + +ChangeStatus AAMemoryBehaviorFunction::manifest(Attributor &A) { + Function &F = cast(getAnchoredValue()); + + // Check if we would improve the existing attributes first + IntegerState ExistingState; + getKnownStateFromValue(F, ExistingState, /* ValueOnly */ true, + /* AAR */ nullptr); + if (ExistingState.isKnown(getAssumed())) + return ChangeStatus::UNCHANGED; + + // Remove existing attributes as they not always mix well with derived ones. + F.removeFnAttr(Attribute::ReadNone); + F.removeFnAttr(Attribute::ReadOnly); + F.removeFnAttr(Attribute::WriteOnly); + if (isAssumedReadNone()) { + F.removeFnAttr(Attribute::ArgMemOnly); + F.removeFnAttr(Attribute::InaccessibleMemOnly); + F.removeFnAttr(Attribute::InaccessibleMemOrArgMemOnly); + } + return AbstractAttribute::manifest(A); +} + /// An AA to represent the memory behavior argument attributes. struct AAMemoryBehaviorArgument final : public AAMemoryBehaviorImpl { /// See AAMemoryBehaviorImpl::AAMemoryBehaviorImpl(...). - AAMemoryBehaviorArgument(Argument &Arg) : AAMemoryBehaviorImpl(Arg) {} + AAMemoryBehaviorArgument(Argument &Arg, AAResults &AAR) + : AAMemoryBehaviorImpl(Arg, AAR) {} /// See AbstractAttribute::initialize(...). void initialize(Attributor &A) override { @@ -1464,7 +1568,7 @@ if (Arg.getNumUses() == 0) addKnownBits(NO_ACCESSES); else - getKnownStateFromValue(Arg, *this, /* ValueOnly */ false); + getKnownStateFromValue(Arg, *this, /* ValueOnly */ false, &AAR); } /// See AbstractAttribute::updateImpl(Attributor &A). @@ -1498,8 +1602,8 @@ // Check if we would improve the existing attributes first. IntegerState ExistingState; - getKnownStateFromValue(Arg, ExistingState, - /* value only */ true); + getKnownStateFromValue(Arg, ExistingState, /* ValueOnly */ true, + /* AAR */ nullptr); if (ExistingState.isKnown(getAssumed())) return ChangeStatus::UNCHANGED; @@ -1545,10 +1649,20 @@ const Instruction *UserI) { assert(UserI->mayReadOrWriteMemory()); + auto IsConstMem = [&](MemoryLocation Loc) { + return AAR.pointsToConstantMemory(Loc, /*OrLocal=*/false); + }; + switch (UserI->getOpcode()) { + case Instruction::VAArg: + // Ignore vaargs on local memory. + if (IsConstMem(MemoryLocation::get(cast(UserI)))) + return; + break; case Instruction::Load: // Loads cause the NO_READS property to disappear. - removeAssumedBits(NO_READS); + if (!IsConstMem(MemoryLocation::get(cast(UserI)))) + removeAssumedBits(NO_READS); return; case Instruction::Store: @@ -1556,7 +1670,8 @@ // pointer operand. Note that we do assume that capturing was taken care of // somewhere else. if (cast(UserI)->getPointerOperand() == U->get()) - removeAssumedBits(NO_WRITES); + if (!IsConstMem(MemoryLocation::get(cast(UserI)))) + removeAssumedBits(NO_WRITES); return; case Instruction::Call: @@ -1571,6 +1686,12 @@ return; } + // FIXME: This should not be neccessary anymore once we have call site + // memory behavior attributes. + FunctionModRefBehavior MRB = AAR.getModRefBehavior(cast(UserI)); + if (MRB == FMRB_DoesNotAccessMemory) + return; + if (ICS.isCallee(U)) break; @@ -1593,7 +1714,7 @@ const Argument &CalleeArg = *(Callee->arg_begin() + ArgNo); IntegerState ExistingState; getKnownStateFromValue(CalleeArg, ExistingState, - /* value only */ false); + /* value only */ false, &AAR); intersectAssumedBits(ExistingState.getKnown()); return; } @@ -1841,6 +1962,8 @@ } void Attributor::identifyAbstractAttributes(Function &F) { + AAResults &AAR = getAAR(F); + // Return attributes are only appropriate if the return type is non void. Type *ReturnType = F.getReturnType(); if (!ReturnType->isVoidTy()) { @@ -1855,6 +1978,9 @@ // Every function might be "no-return". registerAA(*new AANoReturnFunction(F)); + // Every function might be readnone/readonly/writeonly + registerAA(*new AAMemoryBehaviorFunction(F, AAR)); + // For each argument we check if we can derive attributes. for (Argument &Arg : F.args()) { @@ -1862,7 +1988,7 @@ // is also derived but as a "function return attribute" (see above). if (Arg.getType()->isPointerTy()) { registerAA(*new AANoCaptureArgument(Arg)); - registerAA(*new AAMemoryBehaviorArgument(Arg)); + registerAA(*new AAMemoryBehaviorArgument(Arg, AAR)); } } @@ -1946,8 +2072,8 @@ /// ---------------------------------------------------------------------------- PreservedAnalyses AttributorPass::run(LazyCallGraph::SCC &C, - CGSCCAnalysisManager &, LazyCallGraph &, - CGSCCUpdateResult &) { + CGSCCAnalysisManager &AM, + LazyCallGraph &CG, CGSCCUpdateResult &) { SmallVector SCCFunctions; for (LazyCallGraph::Node &N : C) { @@ -1956,7 +2082,14 @@ SCCFunctions.push_back(&Fn); } - Attributor A; + FunctionAnalysisManager &FAM = + AM.getResult(C, CG).getManager(); + + // We pass a lambda into functions to wire them up to the analysis manager + // for getting function analyses. + std::function AARGetter = + [&](Function &F) -> AAResults & { return FAM.getResult(F); }; + Attributor A(AARGetter); if (A.run(SCCFunctions) == ChangeStatus::CHANGED) return PreservedAnalyses::none(); @@ -1983,12 +2116,18 @@ if (!Fn->isDeclaration()) SCCFunctions.push_back(Fn); - Attributor A; + LegacyAARGetter LAARGetter(*this); + std::function AARGetter = + [&](Function &F) -> AAResults & { return LAARGetter(F); }; + + Attributor A(AARGetter); return A.run(SCCFunctions) == ChangeStatus::CHANGED; } void getAnalysisUsage(AnalysisUsage &AU) const override { AU.setPreservesCFG(); + AU.addRequired(); + getAAResultsAnalysisUsage(AU); AU.addPreserved(); CallGraphSCCPass::getAnalysisUsage(AU); } diff --git a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll --- a/llvm/test/Transforms/FunctionAttrs/arg_returned.ll +++ b/llvm/test/Transforms/FunctionAttrs/arg_returned.ll @@ -356,7 +356,7 @@ ; } ; ; FNATTR: define dso_local double @select_and_phi(double %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double @select_and_phi(double returned %b) [[NoInlineNoRecurseNoUnwindUwtable:#[0-9]*]] +; ATTRIBUTOR: define dso_local double @select_and_phi(double returned %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double @select_and_phi(double %b) #0 { entry: @@ -384,7 +384,7 @@ ; } ; ; FNATTR: define dso_local double @recursion_select_and_phi(i32 %a, double %b) [[NoInlineNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double @recursion_select_and_phi(i32 %a, double returned %b) [[NoInlineNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double @recursion_select_and_phi(i32 %a, double returned %b) [[NoInlineNoUnwindReadnoneUwtable]] ; define dso_local double @recursion_select_and_phi(i32 %a, double %b) #0 { entry: @@ -411,7 +411,7 @@ ; } ; ; FNATTR: define dso_local double* @bitcast(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double* @bitcast(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double* @bitcast(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; BOTH: define dso_local double* @bitcast(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double* @bitcast(i32* %b) #0 { @@ -431,7 +431,7 @@ ; } ; ; FNATTR: define dso_local double* @bitcasts_select_and_phi(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double* @bitcasts_select_and_phi(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double* @bitcasts_select_and_phi(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; BOTH: define dso_local double* @bitcasts_select_and_phi(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double* @bitcasts_select_and_phi(i32* %b) #0 { @@ -466,7 +466,7 @@ ; } ; ; FNATTR: define dso_local double* @ret_arg_arg_undef(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double* @ret_arg_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double* @ret_arg_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; BOTH: define dso_local double* @ret_arg_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double* @ret_arg_arg_undef(i32* %b) #0 { @@ -501,7 +501,7 @@ ; } ; ; FNATTR: define dso_local double* @ret_undef_arg_arg(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double* @ret_undef_arg_arg(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double* @ret_undef_arg_arg(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; BOTH: define dso_local double* @ret_undef_arg_arg(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double* @ret_undef_arg_arg(i32* %b) #0 { @@ -536,7 +536,7 @@ ; } ; ; FNATTR: define dso_local double* @ret_undef_arg_undef(i32* readnone %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] -; ATTRIBUTOR: define dso_local double* @ret_undef_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindUwtable]] +; ATTRIBUTOR: define dso_local double* @ret_undef_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; BOTH: define dso_local double* @ret_undef_arg_undef(i32* readnone returned "no-capture-maybe-returned" %b) [[NoInlineNoRecurseNoUnwindReadnoneUwtable]] ; define dso_local double* @ret_undef_arg_undef(i32* %b) #0 {