From 4ebea86ddf20d3f9be3cb363eed3f313591bee66 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Thu, 3 Nov 2022 13:54:08 +0900 Subject: [PATCH] Revert "[AliasAnalysis] Introduce getModRefInfoMask() as a generalization of pointsToConstantMemory()." This reverts commit 01859da84bad95fd51d6a03b08b60c660e642a4f for causing https://github.com/llvm/llvm-project/issues/58776 --- llvm/docs/AliasAnalysis.rst | 26 +++----- llvm/include/llvm/Analysis/AliasAnalysis.h | 51 ++++----------- .../llvm/Analysis/BasicAliasAnalysis.h | 12 +--- .../llvm/Analysis/ObjCARCAliasAnalysis.h | 4 +- .../llvm/Analysis/TypeBasedAliasAnalysis.h | 4 +- llvm/lib/Analysis/AliasAnalysis.cpp | 63 +++++++++---------- llvm/lib/Analysis/BasicAliasAnalysis.cpp | 43 ++++--------- .../lib/Analysis/MemoryDependenceAnalysis.cpp | 2 +- llvm/lib/Analysis/MemorySSA.cpp | 5 +- llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp | 19 +++--- llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp | 14 ++--- .../lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp | 47 ++++++++++++-- llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.h | 4 +- llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 7 +-- .../InstCombine/InstCombineCalls.cpp | 4 +- .../InstCombineLoadStoreAlloca.cpp | 33 +++++----- llvm/lib/Transforms/Scalar/LICM.cpp | 2 +- .../lib/Transforms/Scalar/LoopPredication.cpp | 2 +- llvm/test/Analysis/BasicAA/constant-memory.ll | 14 +++-- .../InstCombine/memcpy-from-global.ll | 6 +- llvm/test/Transforms/InstCombine/store.ll | 4 +- 21 files changed, 168 insertions(+), 198 deletions(-) diff --git a/llvm/docs/AliasAnalysis.rst b/llvm/docs/AliasAnalysis.rst index 046dd24d7332..b9a8a3a4eb52 100644 --- a/llvm/docs/AliasAnalysis.rst +++ b/llvm/docs/AliasAnalysis.rst @@ -161,24 +161,14 @@ Other useful ``AliasAnalysis`` methods Several other tidbits of information are often collected by various alias analysis implementations and can be put to good use by various clients. -The ``getModRefInfoMask`` method -^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -The ``getModRefInfoMask`` method returns a bound on Mod/Ref information for -the supplied pointer, based on knowledge about whether the pointer points to -globally-constant memory (for which it returns ``NoModRef``) or -locally-invariant memory (for which it returns ``Ref``). Globally-constant -memory includes functions, constant global variables, and the null pointer. -Locally-invariant memory is memory that we know is invariant for the lifetime -of its SSA value, but not necessarily for the life of the program: for example, -the memory pointed to by ``readonly`` ``noalias`` parameters is known-invariant -for the duration of the corresponding function call. Given Mod/Ref information -``MRI`` for a memory location ``Loc``, ``MRI`` can be refined with a statement -like ``MRI &= AA.getModRefInfoMask(Loc);``. Another useful idiom is -``isModSet(AA.getModRefInfoMask(Loc))``; this checks to see if the given -location can be modified at all. For convenience, there is also a method -``pointsToConstantMemory(Loc)``; this is synonymous with -``isNoModRef(AA.getModRefInfoMask(Loc))``. +The ``pointsToConstantMemory`` method +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``pointsToConstantMemory`` method returns true if and only if the analysis +can prove that the pointer only points to unchanging memory locations +(functions, constant global variables, and the null pointer). This information +can be used to refine mod/ref information: it is impossible for an unchanging +memory location to be modified. .. _never access memory or only read memory: diff --git a/llvm/include/llvm/Analysis/AliasAnalysis.h b/llvm/include/llvm/Analysis/AliasAnalysis.h index 953e15e358f1..1b15d130967a 100644 --- a/llvm/include/llvm/Analysis/AliasAnalysis.h +++ b/llvm/include/llvm/Analysis/AliasAnalysis.h @@ -375,9 +375,7 @@ public: /// Checks whether the given location points to constant memory, or if /// \p OrLocal is true whether it points to a local alloca. - bool pointsToConstantMemory(const MemoryLocation &Loc, bool OrLocal = false) { - return isNoModRef(getModRefInfoMask(Loc, OrLocal)); - } + bool pointsToConstantMemory(const MemoryLocation &Loc, bool OrLocal = false); /// A convenience wrapper around the primary \c pointsToConstantMemory /// interface. @@ -390,22 +388,6 @@ public: /// \name Simple mod/ref information /// @{ - /// Returns a bitmask that should be unconditionally applied to the ModRef - /// info of a memory location. This allows us to eliminate Mod and/or Ref - /// from the ModRef info based on the knowledge that the memory location - /// points to constant and/or locally-invariant memory. - /// - /// If IgnoreLocals is true, then this method returns NoModRef for memory - /// that points to a local alloca. - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, - bool IgnoreLocals = false); - - /// A convenience wrapper around the primary \c getModRefInfoMask - /// interface. - ModRefInfo getModRefInfoMask(const Value *P, bool IgnoreLocals = false) { - return getModRefInfoMask(MemoryLocation::getBeforeOrAfter(P), IgnoreLocals); - } - /// Get the ModRef info associated with a pointer argument of a call. The /// result's bits are set to indicate the allowed aliasing ModRef kinds. Note /// that these bits do not necessarily account for the overall behavior of @@ -555,8 +537,6 @@ public: bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, bool OrLocal = false); - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals = false); ModRefInfo getModRefInfo(const Instruction *I, const CallBase *Call2, AAQueryInfo &AAQIP); ModRefInfo getModRefInfo(const CallBase *Call, const MemoryLocation &Loc, @@ -624,10 +604,6 @@ public: bool pointsToConstantMemory(const MemoryLocation &Loc, bool OrLocal = false) { return AA.pointsToConstantMemory(Loc, AAQI, OrLocal); } - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, - bool IgnoreLocals = false) { - return AA.getModRefInfoMask(Loc, AAQI, IgnoreLocals); - } ModRefInfo getModRefInfo(const Instruction *I, const std::optional &OptLoc) { return AA.getModRefInfo(I, OptLoc, AAQI); @@ -690,19 +666,16 @@ public: const MemoryLocation &LocB, AAQueryInfo &AAQI, const Instruction *CtxI) = 0; + /// Checks whether the given location points to constant memory, or if + /// \p OrLocal is true whether it points to a local alloca. + virtual bool pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, bool OrLocal) = 0; + /// @} //===--------------------------------------------------------------------===// /// \name Simple mod/ref information /// @{ - /// Returns a bitmask that should be unconditionally applied to the ModRef - /// info of a memory location. This allows us to eliminate Mod and/or Ref from - /// the ModRef info based on the knowledge that the memory location points to - /// constant and/or locally-invariant memory. - virtual ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, - bool IgnoreLocals) = 0; - /// Get the ModRef info associated with a pointer argument of a callsite. The /// result's bits are set to indicate the allowed aliasing ModRef kinds. Note /// that these bits do not necessarily account for the overall behavior of @@ -751,9 +724,9 @@ public: return Result.alias(LocA, LocB, AAQI, CtxI); } - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals) override { - return Result.getModRefInfoMask(Loc, AAQI, IgnoreLocals); + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal) override { + return Result.pointsToConstantMemory(Loc, AAQI, OrLocal); } ModRefInfo getArgModRefInfo(const CallBase *Call, unsigned ArgIdx) override { @@ -806,9 +779,9 @@ public: return AliasResult::MayAlias; } - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals) { - return ModRefInfo::ModRef; + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal) { + return false; } ModRefInfo getArgModRefInfo(const CallBase *Call, unsigned ArgIdx) { diff --git a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h index a2735f039a01..397692694322 100644 --- a/llvm/include/llvm/Analysis/BasicAliasAnalysis.h +++ b/llvm/include/llvm/Analysis/BasicAliasAnalysis.h @@ -73,15 +73,9 @@ public: ModRefInfo getModRefInfo(const CallBase *Call1, const CallBase *Call2, AAQueryInfo &AAQI); - /// Returns a bitmask that should be unconditionally applied to the ModRef - /// info of a memory location. This allows us to eliminate Mod and/or Ref - /// from the ModRef info based on the knowledge that the memory location - /// points to constant and/or locally-invariant memory. - /// - /// If IgnoreLocals is true, then this method returns NoModRef for memory - /// that points to a local alloca. - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals = false); + /// Chases pointers until we find a (constant global) or not. + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal); /// Get the location associated with a pointer argument of a callsite. ModRefInfo getArgModRefInfo(const CallBase *Call, unsigned ArgIdx); diff --git a/llvm/include/llvm/Analysis/ObjCARCAliasAnalysis.h b/llvm/include/llvm/Analysis/ObjCARCAliasAnalysis.h index 1a154c648fe6..ef162ece8a32 100644 --- a/llvm/include/llvm/Analysis/ObjCARCAliasAnalysis.h +++ b/llvm/include/llvm/Analysis/ObjCARCAliasAnalysis.h @@ -52,8 +52,8 @@ public: AliasResult alias(const MemoryLocation &LocA, const MemoryLocation &LocB, AAQueryInfo &AAQI, const Instruction *CtxI); - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals); + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal); using AAResultBase::getMemoryEffects; MemoryEffects getMemoryEffects(const Function *F); diff --git a/llvm/include/llvm/Analysis/TypeBasedAliasAnalysis.h b/llvm/include/llvm/Analysis/TypeBasedAliasAnalysis.h index 36dd39c033aa..84a2a6a2fdac 100644 --- a/llvm/include/llvm/Analysis/TypeBasedAliasAnalysis.h +++ b/llvm/include/llvm/Analysis/TypeBasedAliasAnalysis.h @@ -40,8 +40,8 @@ public: AliasResult alias(const MemoryLocation &LocA, const MemoryLocation &LocB, AAQueryInfo &AAQI, const Instruction *CtxI); - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals); + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal); MemoryEffects getMemoryEffects(const CallBase *Call, AAQueryInfo &AAQI); MemoryEffects getMemoryEffects(const Function *F); diff --git a/llvm/lib/Analysis/AliasAnalysis.cpp b/llvm/lib/Analysis/AliasAnalysis.cpp index 9e24f6b87bdb..5e27808cfdb3 100644 --- a/llvm/lib/Analysis/AliasAnalysis.cpp +++ b/llvm/lib/Analysis/AliasAnalysis.cpp @@ -146,25 +146,19 @@ AliasResult AAResults::alias(const MemoryLocation &LocA, return Result; } -ModRefInfo AAResults::getModRefInfoMask(const MemoryLocation &Loc, - bool IgnoreLocals) { +bool AAResults::pointsToConstantMemory(const MemoryLocation &Loc, + bool OrLocal) { SimpleAAQueryInfo AAQIP(*this); - return getModRefInfoMask(Loc, AAQIP, IgnoreLocals); + return pointsToConstantMemory(Loc, AAQIP, OrLocal); } -ModRefInfo AAResults::getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, bool IgnoreLocals) { - ModRefInfo Result = ModRefInfo::ModRef; - - for (const auto &AA : AAs) { - Result &= AA->getModRefInfoMask(Loc, AAQI, IgnoreLocals); - - // Early-exit the moment we reach the bottom of the lattice. - if (isNoModRef(Result)) - return ModRefInfo::NoModRef; - } +bool AAResults::pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, bool OrLocal) { + for (const auto &AA : AAs) + if (AA->pointsToConstantMemory(Loc, AAQI, OrLocal)) + return true; - return Result; + return false; } ModRefInfo AAResults::getArgModRefInfo(const CallBase *Call, unsigned ArgIdx) { @@ -253,11 +247,10 @@ ModRefInfo AAResults::getModRefInfo(const CallBase *Call, Result &= ArgMR | OtherMR; - // Apply the ModRef mask. This ensures that if Loc is a constant memory - // location, we take into account the fact that the call definitely could not + // If Loc is a constant memory location, the call definitely could not // modify the memory location. - if (!isNoModRef(Result)) - Result &= getModRefInfoMask(Loc); + if (isModSet(Result) && pointsToConstantMemory(Loc, AAQI, /*OrLocal*/ false)) + Result &= ModRefInfo::Ref; return Result; } @@ -495,11 +488,9 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S, if (AR == AliasResult::NoAlias) return ModRefInfo::NoModRef; - // Examine the ModRef mask. If Mod isn't present, then return NoModRef. - // This ensures that if Loc is a constant memory location, we take into - // account the fact that the store definitely could not modify the memory - // location. - if (!isModSet(getModRefInfoMask(Loc))) + // If the pointer is a pointer to constant memory, then it could not have + // been modified by this store. + if (pointsToConstantMemory(Loc, AAQI)) return ModRefInfo::NoModRef; } @@ -510,11 +501,10 @@ ModRefInfo AAResults::getModRefInfo(const StoreInst *S, ModRefInfo AAResults::getModRefInfo(const FenceInst *S, const MemoryLocation &Loc, AAQueryInfo &AAQI) { - // All we know about a fence instruction is what we get from the ModRef - // mask: if Loc is a constant memory location, the fence definitely could - // not modify it. - if (Loc.Ptr) - return getModRefInfoMask(Loc); + // If we know that the location is a constant memory location, the fence + // cannot modify this location. + if (Loc.Ptr && pointsToConstantMemory(Loc, AAQI)) + return ModRefInfo::Ref; return ModRefInfo::ModRef; } @@ -528,9 +518,10 @@ ModRefInfo AAResults::getModRefInfo(const VAArgInst *V, if (AR == AliasResult::NoAlias) return ModRefInfo::NoModRef; - // If the pointer is a pointer to invariant memory, then it could not have + // If the pointer is a pointer to constant memory, then it could not have // been modified by this va_arg. - return getModRefInfoMask(Loc, AAQI); + if (pointsToConstantMemory(Loc, AAQI)) + return ModRefInfo::NoModRef; } // Otherwise, a va_arg reads and writes. @@ -541,9 +532,10 @@ ModRefInfo AAResults::getModRefInfo(const CatchPadInst *CatchPad, const MemoryLocation &Loc, AAQueryInfo &AAQI) { if (Loc.Ptr) { - // If the pointer is a pointer to invariant memory, + // If the pointer is a pointer to constant memory, // then it could not have been modified by this catchpad. - return getModRefInfoMask(Loc, AAQI); + if (pointsToConstantMemory(Loc, AAQI)) + return ModRefInfo::NoModRef; } // Otherwise, a catchpad reads and writes. @@ -554,9 +546,10 @@ ModRefInfo AAResults::getModRefInfo(const CatchReturnInst *CatchRet, const MemoryLocation &Loc, AAQueryInfo &AAQI) { if (Loc.Ptr) { - // If the pointer is a pointer to invariant memory, + // If the pointer is a pointer to constant memory, // then it could not have been modified by this catchpad. - return getModRefInfoMask(Loc, AAQI); + if (pointsToConstantMemory(Loc, AAQI)) + return ModRefInfo::NoModRef; } // Otherwise, a catchret reads and writes. diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp index 1ea1d4196f80..edf46664139e 100644 --- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp +++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp @@ -678,46 +678,33 @@ BasicAAResult::DecomposeGEPExpression(const Value *V, const DataLayout &DL, return Decomposed; } -ModRefInfo BasicAAResult::getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, - bool IgnoreLocals) { +/// Returns whether the given pointer value points to memory that is local to +/// the function, with global constants being considered local to all +/// functions. +bool BasicAAResult::pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, bool OrLocal) { assert(Visited.empty() && "Visited must be cleared after use!"); - auto _ = make_scope_exit([&] { Visited.clear(); }); + auto _ = make_scope_exit([&]{ Visited.clear(); }); unsigned MaxLookup = 8; SmallVector Worklist; Worklist.push_back(Loc.Ptr); - ModRefInfo Result = ModRefInfo::NoModRef; - do { const Value *V = getUnderlyingObject(Worklist.pop_back_val()); if (!Visited.insert(V).second) continue; - // Ignore allocas if we were instructed to do so. - if (IgnoreLocals && isa(V)) + // An alloca instruction defines local memory. + if (OrLocal && isa(V)) continue; - // If the location points to memory that is known to be invariant for - // the life of the underlying SSA value, then we can exclude Mod from - // the set of valid memory effects. - // - // An argument that is marked readonly and noalias is known to be - // invariant while that function is executing. - if (const Argument *Arg = dyn_cast(V)) { - if (Arg->hasNoAliasAttr() && Arg->onlyReadsMemory()) { - Result |= ModRefInfo::Ref; - continue; - } - } - - // A global constant can't be mutated. + // A global constant counts as local memory for our purposes. if (const GlobalVariable *GV = dyn_cast(V)) { // Note: this doesn't require GV to be "ODR" because it isn't legal for a // global to be marked constant in some modules and non-constant in // others. GV may even be a declaration, not a definition. if (!GV->isConstant()) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); continue; } @@ -733,20 +720,16 @@ ModRefInfo BasicAAResult::getModRefInfoMask(const MemoryLocation &Loc, if (const PHINode *PN = dyn_cast(V)) { // Don't bother inspecting phi nodes with many operands. if (PN->getNumIncomingValues() > MaxLookup) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); append_range(Worklist, PN->incoming_values()); continue; } // Otherwise be conservative. - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); } while (!Worklist.empty() && --MaxLookup); - // If we hit the maximum number of instructions to examine, be conservative. - if (!Worklist.empty()) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); - - return Result; + return Worklist.empty(); } static bool isIntrinsicCall(const CallBase *Call, Intrinsic::ID IID) { diff --git a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp index 2340015d6517..6a0b123f0b8f 100644 --- a/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp +++ b/llvm/lib/Analysis/MemoryDependenceAnalysis.cpp @@ -524,7 +524,7 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom( } // Stores don't alias loads from read-only memory. - if (!isModSet(BatchAA.getModRefInfoMask(LoadLoc))) + if (BatchAA.pointsToConstantMemory(LoadLoc)) continue; // Stores depend on may/must aliased loads. diff --git a/llvm/lib/Analysis/MemorySSA.cpp b/llvm/lib/Analysis/MemorySSA.cpp index 0e6a12261e23..c2e2c8d9ec0f 100644 --- a/llvm/lib/Analysis/MemorySSA.cpp +++ b/llvm/lib/Analysis/MemorySSA.cpp @@ -375,10 +375,9 @@ static bool isUseTriviallyOptimizableToLiveOnEntry(BatchAAResults &AA, const Instruction *I) { // If the memory can't be changed, then loads of the memory can't be // clobbered. - if (auto *LI = dyn_cast(I)) { + if (auto *LI = dyn_cast(I)) return I->hasMetadata(LLVMContext::MD_invariant_load) || - !isModSet(AA.getModRefInfoMask(MemoryLocation::get(LI))); - } + AA.pointsToConstantMemory(MemoryLocation::get(LI)); return false; } diff --git a/llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp b/llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp index 1ccf792d2f8c..dd9eae428b0a 100644 --- a/llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp +++ b/llvm/lib/Analysis/ObjCARCAliasAnalysis.cpp @@ -69,29 +69,28 @@ AliasResult ObjCARCAAResult::alias(const MemoryLocation &LocA, return AliasResult::MayAlias; } -ModRefInfo ObjCARCAAResult::getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, - bool IgnoreLocals) { +bool ObjCARCAAResult::pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, bool OrLocal) { if (!EnableARCOpts) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); // First, strip off no-ops, including ObjC-specific no-ops, and try making // a precise alias query. const Value *S = GetRCIdentityRoot(Loc.Ptr); - if (isNoModRef(AAResultBase::getModRefInfoMask( - MemoryLocation(S, Loc.Size, Loc.AATags), AAQI, IgnoreLocals))) - return ModRefInfo::NoModRef; + if (AAResultBase::pointsToConstantMemory( + MemoryLocation(S, Loc.Size, Loc.AATags), AAQI, OrLocal)) + return true; // If that failed, climb to the underlying object, including climbing through // ObjC-specific no-ops, and try making an imprecise alias query. const Value *U = GetUnderlyingObjCPtr(S); if (U != S) - return AAResultBase::getModRefInfoMask(MemoryLocation::getBeforeOrAfter(U), - AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory( + MemoryLocation::getBeforeOrAfter(U), AAQI, OrLocal); // If that failed, fail. We don't need to chain here, since that's covered // by the earlier precise query. - return ModRefInfo::ModRef; + return false; } MemoryEffects ObjCARCAAResult::getMemoryEffects(const Function *F) { diff --git a/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp b/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp index 529f3a76d23e..077fbdbe821c 100644 --- a/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp +++ b/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp @@ -385,23 +385,23 @@ AliasResult TypeBasedAAResult::alias(const MemoryLocation &LocA, return AliasResult::NoAlias; } -ModRefInfo TypeBasedAAResult::getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, - bool IgnoreLocals) { +bool TypeBasedAAResult::pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, + bool OrLocal) { if (!EnableTBAA) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); const MDNode *M = Loc.AATags.TBAA; if (!M) - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); // If this is an "immutable" type, we can assume the pointer is pointing // to constant memory. if ((!isStructPathTBAA(M) && TBAANode(M).isTypeImmutable()) || (isStructPathTBAA(M) && TBAAStructTagNode(M).isTypeImmutable())) - return ModRefInfo::NoModRef; + return true; - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); } MemoryEffects TypeBasedAAResult::getMemoryEffects(const CallBase *Call, diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp index 8155c895e366..1dd91f2b0be8 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.cpp @@ -124,19 +124,54 @@ AliasResult AMDGPUAAResult::alias(const MemoryLocation &LocA, return AAResultBase::alias(LocA, LocB, AAQI, nullptr); } -ModRefInfo AMDGPUAAResult::getModRefInfoMask(const MemoryLocation &Loc, - AAQueryInfo &AAQI, - bool IgnoreLocals) { +bool AMDGPUAAResult::pointsToConstantMemory(const MemoryLocation &Loc, + AAQueryInfo &AAQI, bool OrLocal) { unsigned AS = Loc.Ptr->getType()->getPointerAddressSpace(); if (AS == AMDGPUAS::CONSTANT_ADDRESS || AS == AMDGPUAS::CONSTANT_ADDRESS_32BIT) - return ModRefInfo::NoModRef; + return true; const Value *Base = getUnderlyingObject(Loc.Ptr); AS = Base->getType()->getPointerAddressSpace(); if (AS == AMDGPUAS::CONSTANT_ADDRESS || AS == AMDGPUAS::CONSTANT_ADDRESS_32BIT) - return ModRefInfo::NoModRef; + return true; + + if (const GlobalVariable *GV = dyn_cast(Base)) { + if (GV->isConstant()) + return true; + } else if (const Argument *Arg = dyn_cast(Base)) { + const Function *F = Arg->getParent(); + + // Only assume constant memory for arguments on kernels. + switch (F->getCallingConv()) { + default: + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); + case CallingConv::AMDGPU_LS: + case CallingConv::AMDGPU_HS: + case CallingConv::AMDGPU_ES: + case CallingConv::AMDGPU_GS: + case CallingConv::AMDGPU_VS: + case CallingConv::AMDGPU_PS: + case CallingConv::AMDGPU_CS: + case CallingConv::AMDGPU_KERNEL: + case CallingConv::SPIR_KERNEL: + break; + } - return AAResultBase::getModRefInfoMask(Loc, AAQI, IgnoreLocals); + unsigned ArgNo = Arg->getArgNo(); + /* On an argument, ReadOnly attribute indicates that the function does + not write through this pointer argument, even though it may write + to the memory that the pointer points to. + On an argument, ReadNone attribute indicates that the function does + not dereference that pointer argument, even though it may read or write + the memory that the pointer points to if accessed through other pointers. + */ + if (F->hasParamAttribute(ArgNo, Attribute::NoAlias) && + (F->hasParamAttribute(ArgNo, Attribute::ReadNone) || + F->hasParamAttribute(ArgNo, Attribute::ReadOnly))) { + return true; + } + } + return AAResultBase::pointsToConstantMemory(Loc, AAQI, OrLocal); } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.h b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.h index 8ce7000222fa..140ed12a8f6d 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.h +++ b/llvm/lib/Target/AMDGPU/AMDGPUAliasAnalysis.h @@ -38,8 +38,8 @@ public: AliasResult alias(const MemoryLocation &LocA, const MemoryLocation &LocB, AAQueryInfo &AAQI, const Instruction *CtxI); - ModRefInfo getModRefInfoMask(const MemoryLocation &Loc, AAQueryInfo &AAQI, - bool IgnoreLocals); + bool pointsToConstantMemory(const MemoryLocation &Loc, AAQueryInfo &AAQI, + bool OrLocal); }; /// Analysis pass providing a never-invalidated alias analysis result. diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp index 3f61dbe3354e..cfbceebf66d4 100644 --- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp +++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp @@ -138,14 +138,13 @@ static MemoryEffects checkFunctionMemoryAccess(Function &F, bool ThisBody, ME |= MemoryEffects::argMemOnly(ModRefInfo::ModRef); auto AddLocAccess = [&](const MemoryLocation &Loc, ModRefInfo MR) { - // Ignore accesses to known-invariant or local memory. - MR &= AAR.getModRefInfoMask(Loc, /*IgnoreLocal=*/true); - if (isNoModRef(MR)) + // Ignore accesses to local memory. + if (AAR.pointsToConstantMemory(Loc, /*OrLocal=*/true)) return; const Value *UO = getUnderlyingObject(Loc.Ptr); assert(!isa(UO) && - "Should have been handled by getModRefInfoMask()"); + "Should have been handled by pointsToConstantMemory()"); if (isa(UO)) { ME |= MemoryEffects::argMemOnly(MR); return; diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index bd3db4534a85..8fad7e8195c8 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -135,7 +135,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemTransfer(AnyMemTransferInst *MI) { // If we have a store to a location which is known constant, we can conclude // that the store must be storing the constant value (else the memory // wouldn't be constant), and this must be a noop. - if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) { + if (AA->pointsToConstantMemory(MI->getDest())) { // Set the size of the copy to 0, it will be deleted on the next iteration. MI->setLength(Constant::getNullValue(MI->getLength()->getType())); return MI; @@ -253,7 +253,7 @@ Instruction *InstCombinerImpl::SimplifyAnyMemSet(AnyMemSetInst *MI) { // If we have a store to a location which is known constant, we can conclude // that the store must be storing the constant value (else the memory // wouldn't be constant), and this must be a noop. - if (!isModSet(AA->getModRefInfoMask(MI->getDest()))) { + if (AA->pointsToConstantMemory(MI->getDest())) { // Set the size of the copy to 0, it will be deleted on the next iteration. MI->setLength(Constant::getNullValue(MI->getLength()->getType())); return MI; diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp index c8226ce2816a..8d58241895b5 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp @@ -36,20 +36,20 @@ static cl::opt MaxCopiedFromConstantUsers( cl::desc("Maximum users to visit in copy from constant transform"), cl::Hidden); -/// isOnlyCopiedFromConstantMemory - Recursively walk the uses of a (derived) +/// isOnlyCopiedFromConstantGlobal - Recursively walk the uses of a (derived) /// pointer to an alloca. Ignore any reads of the pointer, return false if we /// see any stores or other unknown uses. If we see pointer arithmetic, keep /// track of whether it moves the pointer (with IsOffset) but otherwise traverse /// the uses. If we see a memcpy/memmove that targets an unoffseted pointer to -/// the alloca, and if the source pointer is a pointer to a constant memory -/// location, we can optimize this. +/// the alloca, and if the source pointer is a pointer to a constant global, we +/// can optimize this. static bool isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V, MemTransferInst *&TheCopy, SmallVectorImpl &ToDelete) { // We track lifetime intrinsics as we encounter them. If we decide to go - // ahead and replace the value with the memory location, this lets the caller - // quickly eliminate the markers. + // ahead and replace the value with the global, this lets the caller quickly + // eliminate the markers. using ValueAndIsOffset = PointerIntPair; SmallVector Worklist; @@ -150,8 +150,8 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V, // If the memintrinsic isn't using the alloca as the dest, reject it. if (U.getOperandNo() != 0) return false; - // If the source of the memcpy/move is not constant, reject it. - if (isModSet(AA->getModRefInfoMask(MI->getSource()))) + // If the source of the memcpy/move is not a constant global, reject it. + if (!AA->pointsToConstantMemory(MI->getSource())) return false; // Otherwise, the transform is safe. Remember the copy instruction. @@ -161,10 +161,9 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V, return true; } -/// isOnlyCopiedFromConstantMemory - Return true if the specified alloca is only -/// modified by a copy from a constant memory location. If we can prove this, we -/// can replace any uses of the alloca with uses of the memory location -/// directly. +/// isOnlyCopiedFromConstantGlobal - Return true if the specified alloca is only +/// modified by a copy from a constant global. If we can prove this, we can +/// replace any uses of the alloca with uses of the global directly. static MemTransferInst * isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *AI, @@ -420,11 +419,11 @@ Instruction *InstCombinerImpl::visitAllocaInst(AllocaInst &AI) { } // Check to see if this allocation is only modified by a memcpy/memmove from - // a memory location whose alignment is equal to or exceeds that of the - // allocation. If this is the case, we can change all users to use the - // constant memory location instead. This is commonly produced by the CFE by - // constructs like "void foo() { int A[] = {1,2,3,4,5,6,7,8,9...}; }" if 'A' - // is only subsequently read. + // a constant whose alignment is equal to or exceeds that of the allocation. + // If this is the case, we can change all users to use the constant global + // instead. This is commonly produced by the CFE by constructs like "void + // foo() { int A[] = {1,2,3,4,5,6,7,8,9...}; }" if 'A' is only subsequently + // read. SmallVector ToDelete; if (MemTransferInst *Copy = isOnlyCopiedFromConstantMemory(AA, &AI, ToDelete)) { Value *TheSrc = Copy->getSource(); @@ -1401,7 +1400,7 @@ Instruction *InstCombinerImpl::visitStoreInst(StoreInst &SI) { // If we have a store to a location which is known constant, we can conclude // that the store must be storing the constant value (else the memory // wouldn't be constant), and this must be a noop. - if (!isModSet(AA->getModRefInfoMask(Ptr))) + if (AA->pointsToConstantMemory(Ptr)) return eraseInstFromFunction(SI); // Do really simple DSE, to catch cases where there are several consecutive diff --git a/llvm/lib/Transforms/Scalar/LICM.cpp b/llvm/lib/Transforms/Scalar/LICM.cpp index d3739f31bc57..11b96af0a30e 100644 --- a/llvm/lib/Transforms/Scalar/LICM.cpp +++ b/llvm/lib/Transforms/Scalar/LICM.cpp @@ -1160,7 +1160,7 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, // Loads from constant memory are always safe to move, even if they end up // in the same alias set as something that ends up being modified. - if (!isModSet(AA->getModRefInfoMask(LI->getOperand(0)))) + if (AA->pointsToConstantMemory(LI->getOperand(0))) return true; if (LI->hasMetadata(LLVMContext::MD_invariant_load)) return true; diff --git a/llvm/lib/Transforms/Scalar/LoopPredication.cpp b/llvm/lib/Transforms/Scalar/LoopPredication.cpp index 0492ed5cdb1c..8dd9f0144941 100644 --- a/llvm/lib/Transforms/Scalar/LoopPredication.cpp +++ b/llvm/lib/Transforms/Scalar/LoopPredication.cpp @@ -570,7 +570,7 @@ bool LoopPredication::isLoopInvariantValue(const SCEV* S) { if (const SCEVUnknown *U = dyn_cast(S)) if (const auto *LI = dyn_cast(U->getValue())) if (LI->isUnordered() && L->hasLoopInvariantOperands(LI)) - if (!isModSet(AA->getModRefInfoMask(LI->getOperand(0))) || + if (AA->pointsToConstantMemory(LI->getOperand(0)) || LI->hasMetadata(LLVMContext::MD_invariant_load)) return true; return false; diff --git a/llvm/test/Analysis/BasicAA/constant-memory.ll b/llvm/test/Analysis/BasicAA/constant-memory.ll index 2b197d6dbc71..6ef875d9358e 100644 --- a/llvm/test/Analysis/BasicAA/constant-memory.ll +++ b/llvm/test/Analysis/BasicAA/constant-memory.ll @@ -6,16 +6,18 @@ declare void @dummy() declare void @foo(ptr) +; FIXME: This could be NoModRef ; CHECK-LABEL: Function: basic -; CHECK: NoModRef: Ptr: i32* @c <-> call void @dummy() +; CHECK: Just Ref: Ptr: i32* @c <-> call void @dummy() define void @basic(ptr %p) { call void @dummy() load i32, ptr @c ret void } +; FIXME: This could be NoModRef ; CHECK-LABEL: Function: recphi -; CHECK: NoModRef: Ptr: i32* %p <-> call void @dummy() +; CHECK: Just Ref: Ptr: i32* %p <-> call void @dummy() define void @recphi() { entry: br label %loop @@ -32,20 +34,20 @@ exit: ret void } -; Tests that readonly noalias implies !Mod. +; Tests that readonly noalias doesn't imply !Mod yet. ; ; CHECK-LABEL: Function: readonly_noalias -; CHECK: Just Ref: Ptr: i32* %p <-> call void @foo(ptr %p) +; CHECK: Both ModRef: Ptr: i32* %p <-> call void @foo(ptr %p) define void @readonly_noalias(ptr readonly noalias %p) { call void @foo(ptr %p) load i32, ptr %p ret void } -; Tests that readnone noalias implies !Mod. +; Tests that readnone noalias doesn't imply !Mod yet. ; ; CHECK-LABEL: Function: readnone_noalias -; CHECK: Just Ref: Ptr: i32* %p <-> call void @foo(ptr %p) +; CHECK: Both ModRef: Ptr: i32* %p <-> call void @foo(ptr %p) define void @readnone_noalias(ptr readnone noalias %p) { call void @foo(ptr %p) load i32, ptr %p diff --git a/llvm/test/Transforms/InstCombine/memcpy-from-global.ll b/llvm/test/Transforms/InstCombine/memcpy-from-global.ll index b230c1488f75..cd6dfc4561b9 100644 --- a/llvm/test/Transforms/InstCombine/memcpy-from-global.ll +++ b/llvm/test/Transforms/InstCombine/memcpy-from-global.ll @@ -338,10 +338,12 @@ entry: ret float %r } -; Tests that we can eliminate allocas copied from readonly noalias pointers. +; Tests that we can't eliminate allocas copied from readonly noalias pointers yet. define void @memcpy_from_readonly_noalias(ptr readonly noalias align 8 dereferenceable(124) %arg) { ; CHECK-LABEL: @memcpy_from_readonly_noalias( -; CHECK-NEXT: call void @bar(ptr nonnull [[ARG:%.*]]) #[[ATTR3]] +; CHECK-NEXT: [[ALLOCA:%.*]] = alloca [[T:%.*]], align 8 +; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr noundef nonnull align 8 dereferenceable(124) [[ALLOCA]], ptr noundef nonnull align 8 dereferenceable(124) [[ARG:%.*]], i64 124, i1 false) +; CHECK-NEXT: call void @bar(ptr nonnull [[ALLOCA]]) #[[ATTR3]] ; CHECK-NEXT: ret void ; %alloca = alloca %T, align 8 diff --git a/llvm/test/Transforms/InstCombine/store.ll b/llvm/test/Transforms/InstCombine/store.ll index 95ba64c9e640..7cbe91b44beb 100644 --- a/llvm/test/Transforms/InstCombine/store.ll +++ b/llvm/test/Transforms/InstCombine/store.ll @@ -336,10 +336,12 @@ define void @store_to_constant() { ret void } -; Delete stores to readonly noalias pointers. +; We can't delete stores to readonly noalias pointers yet. define void @store_to_readonly_noalias(ptr readonly noalias %0) { ; CHECK-LABEL: @store_to_readonly_noalias( +; CHECK-NEXT: store i32 3, ptr [[TMP0:%.*]], align 4 ; CHECK-NEXT: ret void +; store i32 3, ptr %0, align 4 ret void } -- 2.38.1.1.g6d9df9d320