From 9fc47e681e1d28af538edc54101c9d84e5b5d54b Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 16 Aug 2018 20:11:15 +0000 Subject: [PATCH] [LICM][NFC] Restructure pointer invalidation API in terms of MemoryLocation Main value is just simplifying code. I'll further simply the argument handling case in a bit, but that involved a slightly orthogonal change so I went with the mildy ugly intermediate for this patch. Note that the isSized check in the old LICM code was not carried across. It turns out that check was dead. a) no test exercised it, and b) langref and verifier had been updated to disallow unsized types used in loads. llvm-svn: 339930 --- include/llvm/Analysis/AliasSetTracker.h | 18 +++++---------- lib/Analysis/AliasSetTracker.cpp | 13 ++++++----- lib/Transforms/Scalar/LICM.cpp | 29 +++++++++---------------- 3 files changed, 22 insertions(+), 38 deletions(-) diff --git a/include/llvm/Analysis/AliasSetTracker.h b/include/llvm/Analysis/AliasSetTracker.h index 84dda36e866..106b973eeb8 100644 --- a/include/llvm/Analysis/AliasSetTracker.h +++ b/include/llvm/Analysis/AliasSetTracker.h @@ -393,19 +393,11 @@ public: /// Return the alias sets that are active. const ilist &getAliasSets() const { return AliasSets; } - /// Return the alias set that the specified pointer lives in. If the New - /// argument is non-null, this method sets the value to true if a new alias - /// set is created to contain the pointer (because the pointer didn't alias - /// anything). - AliasSet &getAliasSetForPointer(Value *P, LocationSize Size, - const AAMDNodes &AAInfo); - - /// Return the alias set containing the location specified if one exists, - /// otherwise return null. - AliasSet *getAliasSetForPointerIfExists(const Value *P, LocationSize Size, - const AAMDNodes &AAInfo) { - return mergeAliasSetsForPointer(P, Size, AAInfo); - } + /// Return the alias set which contains the specified memory location. If + /// the memory location aliases two or more existing alias sets, will have + /// the effect of merging those alias sets before the single resulting alias + /// set is returned. + AliasSet &getAliasSetFor(const MemoryLocation &MemLoc); /// Return true if the specified instruction "may" (or must) alias one of the /// members in any of the sets. diff --git a/lib/Analysis/AliasSetTracker.cpp b/lib/Analysis/AliasSetTracker.cpp index 1a495ec3000..2879dea9f58 100644 --- a/lib/Analysis/AliasSetTracker.cpp +++ b/lib/Analysis/AliasSetTracker.cpp @@ -307,11 +307,12 @@ AliasSet *AliasSetTracker::findAliasSetForUnknownInst(Instruction *Inst) { return FoundSet; } -/// getAliasSetForPointer - Return the alias set that the specified pointer -/// lives in. -AliasSet &AliasSetTracker::getAliasSetForPointer(Value *Pointer, - LocationSize Size, - const AAMDNodes &AAInfo) { +AliasSet &AliasSetTracker::getAliasSetFor(const MemoryLocation &MemLoc) { + + Value * const Pointer = const_cast(MemLoc.Ptr); + const LocationSize Size = MemLoc.Size; + const AAMDNodes &AAInfo = MemLoc.AATags; + AliasSet::PointerRec &Entry = getEntryFor(Pointer); if (AliasAnyAS) { @@ -567,7 +568,7 @@ AliasSet &AliasSetTracker::mergeAllAliasSets() { AliasSet &AliasSetTracker::addPointer(Value *P, LocationSize Size, const AAMDNodes &AAInfo, AliasSet::AccessLattice E) { - AliasSet &AS = getAliasSetForPointer(P, Size, AAInfo); + AliasSet &AS = getAliasSetFor(MemoryLocation(P, Size, AAInfo)); AS.Access |= E; if (!AliasAnyAS && (TotalMayAliasSetSize > SaturationThreshold)) { diff --git a/lib/Transforms/Scalar/LICM.cpp b/lib/Transforms/Scalar/LICM.cpp index 79136a97905..f576661363d 100644 --- a/lib/Transforms/Scalar/LICM.cpp +++ b/lib/Transforms/Scalar/LICM.cpp @@ -105,9 +105,8 @@ static bool isSafeToExecuteUnconditionally(Instruction &Inst, const LoopSafetyInfo *SafetyInfo, OptimizationRemarkEmitter *ORE, const Instruction *CtxI = nullptr); -static bool pointerInvalidatedByLoop(Value *V, uint64_t Size, - const AAMDNodes &AAInfo, - AliasSetTracker *CurAST); +static bool isInvalidatedByLoop(const MemoryLocation &MemLoc, + AliasSetTracker *CurAST); static Instruction * CloneInstructionInExitBlock(Instruction &I, BasicBlock &ExitBlock, PHINode &PN, const LoopInfo *LI, @@ -629,16 +628,7 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, if (isLoadInvariantInLoop(LI, DT, CurLoop)) return true; - // Don't hoist loads which have may-aliased stores in loop. - uint64_t Size = 0; - if (LI->getType()->isSized()) - Size = I.getModule()->getDataLayout().getTypeStoreSize(LI->getType()); - - AAMDNodes AAInfo; - LI->getAAMetadata(AAInfo); - - bool Invalidated = - pointerInvalidatedByLoop(LI->getOperand(0), Size, AAInfo, CurAST); + bool Invalidated = isInvalidatedByLoop(MemoryLocation::get(LI), CurAST); // Check loop-invariant address because this may also be a sinkable load // whose address is not necessarily loop-invariant. if (ORE && Invalidated && CurLoop->isLoopInvariant(LI->getPointerOperand())) @@ -679,8 +669,10 @@ bool llvm::canSinkOrHoistInst(Instruction &I, AAResults *AA, DominatorTree *DT, if (AliasAnalysis::onlyAccessesArgPointees(Behavior)) { for (Value *Op : CI->arg_operands()) if (Op->getType()->isPointerTy() && - pointerInvalidatedByLoop(Op, MemoryLocation::UnknownSize, - AAMDNodes(), CurAST)) + isInvalidatedByLoop(MemoryLocation(Op, + MemoryLocation::UnknownSize, + AAMDNodes()), + CurAST)) return false; return true; } @@ -1580,11 +1572,10 @@ void LegacyLICMPass::deleteAnalysisLoop(Loop *L) { /// Return true if the body of this loop may store into the memory /// location pointed to by V. /// -static bool pointerInvalidatedByLoop(Value *V, uint64_t Size, - const AAMDNodes &AAInfo, - AliasSetTracker *CurAST) { +static bool isInvalidatedByLoop(const MemoryLocation &MemLoc, + AliasSetTracker *CurAST) { // Check to see if any of the basic blocks in CurLoop invalidate *V. - return CurAST->getAliasSetForPointer(V, Size, AAInfo).isMod(); + return CurAST->getAliasSetFor(MemLoc).isMod(); } /// Little predicate that returns true if the specified basic block is in