[llvm] r224247 - Sink store based on alias analysis
David Majnemer
david.majnemer at gmail.com
Tue Feb 17 00:42:38 PST 2015
Sorry to revive this thread but reverting this commit seems to fix PR22613:
http://llvm.org/bugs/show_bug.cgi?id=22613.
On Mon, Dec 15, 2014 at 6:09 AM, Elena Demikhovsky <
elena.demikhovsky at intel.com> wrote:
> Author: delena
> Date: Mon Dec 15 08:09:53 2014
> New Revision: 224247
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224247&view=rev
> Log:
> Sink store based on alias analysis
> - by Ella Bolshinsky
> The alias analysis is used define whether the given instruction
> is a barrier for store sinking. For 2 identical stores, following
> instructions are checked in the both basic blocks, to determine
> whether they are sinking barriers.
>
> http://reviews.llvm.org/D6420
>
>
> Modified:
> llvm/trunk/include/llvm/Analysis/AliasAnalysis.h
> llvm/trunk/lib/Analysis/AliasAnalysis.cpp
> llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
> llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
>
> Modified: llvm/trunk/include/llvm/Analysis/AliasAnalysis.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/AliasAnalysis.h?rev=224247&r1=224246&r2=224247&view=diff
>
> ==============================================================================
> --- llvm/trunk/include/llvm/Analysis/AliasAnalysis.h (original)
> +++ llvm/trunk/include/llvm/Analysis/AliasAnalysis.h Mon Dec 15 08:09:53
> 2014
> @@ -502,7 +502,7 @@ public:
> ///
>
> /// canBasicBlockModify - Return true if it is possible for execution
> of the
> - /// specified basic block to modify the value pointed to by Ptr.
> + /// specified basic block to modify the location Loc.
> bool canBasicBlockModify(const BasicBlock &BB, const Location &Loc);
>
> /// canBasicBlockModify - A convenience wrapper.
> @@ -510,17 +510,20 @@ public:
> return canBasicBlockModify(BB, Location(P, Size));
> }
>
> - /// canInstructionRangeModify - Return true if it is possible for the
> - /// execution of the specified instructions to modify the value pointed
> to by
> - /// Ptr. The instructions to consider are all of the instructions in
> the
> - /// range of [I1,I2] INCLUSIVE. I1 and I2 must be in the same basic
> block.
> - bool canInstructionRangeModify(const Instruction &I1, const Instruction
> &I2,
> - const Location &Loc);
> + /// canInstructionRangeModRef - Return true if it is possible for the
> + /// execution of the specified instructions to mod\ref (according to the
> + /// mode) the location Loc. The instructions to consider are all
> + /// of the instructions in the range of [I1,I2] INCLUSIVE.
> + /// I1 and I2 must be in the same basic block.
> + bool canInstructionRangeModRef(const Instruction &I1,
> + const Instruction &I2, const Location
> &Loc,
> + const ModRefResult Mode);
>
> - /// canInstructionRangeModify - A convenience wrapper.
> - bool canInstructionRangeModify(const Instruction &I1, const Instruction
> &I2,
> - const Value *Ptr, uint64_t Size) {
> - return canInstructionRangeModify(I1, I2, Location(Ptr, Size));
> + /// canInstructionRangeModRef - A convenience wrapper.
> + bool canInstructionRangeModRef(const Instruction &I1,
> + const Instruction &I2, const Value *Ptr,
> + uint64_t Size, const ModRefResult Mode) {
> + return canInstructionRangeModRef(I1, I2, Location(Ptr, Size), Mode);
> }
>
>
> //===--------------------------------------------------------------------===//
>
> Modified: llvm/trunk/lib/Analysis/AliasAnalysis.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/AliasAnalysis.cpp?rev=224247&r1=224246&r2=224247&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/AliasAnalysis.cpp (original)
> +++ llvm/trunk/lib/Analysis/AliasAnalysis.cpp Mon Dec 15 08:09:53 2014
> @@ -483,21 +483,22 @@ uint64_t AliasAnalysis::getTypeStoreSize
> }
>
> /// canBasicBlockModify - Return true if it is possible for execution of
> the
> -/// specified basic block to modify the value pointed to by Ptr.
> +/// specified basic block to modify the location Loc.
> ///
> bool AliasAnalysis::canBasicBlockModify(const BasicBlock &BB,
> const Location &Loc) {
> - return canInstructionRangeModify(BB.front(), BB.back(), Loc);
> + return canInstructionRangeModRef(BB.front(), BB.back(), Loc, Mod);
> }
>
> -/// canInstructionRangeModify - Return true if it is possible for the
> execution
> -/// of the specified instructions to modify the value pointed to by Ptr.
> The
> -/// instructions to consider are all of the instructions in the range of
> [I1,I2]
> -/// INCLUSIVE. I1 and I2 must be in the same basic block.
> -///
> -bool AliasAnalysis::canInstructionRangeModify(const Instruction &I1,
> +/// canInstructionRangeModRef - Return true if it is possible for the
> +/// execution of the specified instructions to mod\ref (according to the
> +/// mode) the location Loc. The instructions to consider are all
> +/// of the instructions in the range of [I1,I2] INCLUSIVE.
> +/// I1 and I2 must be in the same basic block.
> +bool AliasAnalysis::canInstructionRangeModRef(const Instruction &I1,
> const Instruction &I2,
> - const Location &Loc) {
> + const Location &Loc,
> + const ModRefResult Mode) {
> assert(I1.getParent() == I2.getParent() &&
> "Instructions not in same basic block!");
> BasicBlock::const_iterator I = &I1;
> @@ -505,7 +506,7 @@ bool AliasAnalysis::canInstructionRangeM
> ++E; // Convert from inclusive to exclusive range.
>
> for (; I != E; ++I) // Check every instruction in range
> - if (getModRefInfo(I, Loc) & Mod)
> + if (getModRefInfo(I, Loc) & Mode)
> return true;
> return false;
> }
>
> Modified: llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp?rev=224247&r1=224246&r2=224247&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp (original)
> +++ llvm/trunk/lib/Transforms/IPO/ArgumentPromotion.cpp Mon Dec 15
> 08:09:53 2014
> @@ -554,7 +554,8 @@ bool ArgPromotion::isSafeToPromoteArgume
> BasicBlock *BB = Load->getParent();
>
> AliasAnalysis::Location Loc = AA.getLocation(Load);
> - if (AA.canInstructionRangeModify(BB->front(), *Load, Loc))
> + if (AA.canInstructionRangeModRef(BB->front(), *Load, Loc,
> + AliasAnalysis::Mod))
> return false; // Pointer is invalidated!
>
> // Now check every path from the entry block to the load for
> transparency.
>
> Modified: llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp?rev=224247&r1=224246&r2=224247&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/MergedLoadStoreMotion.cpp Mon Dec 15
> 08:09:53 2014
> @@ -143,7 +143,9 @@ private:
> // Routines for sinking stores
> StoreInst *canSinkFromBlock(BasicBlock *BB, StoreInst *SI);
> PHINode *getPHIOperand(BasicBlock *BB, StoreInst *S0, StoreInst *S1);
> - bool isStoreSinkBarrier(Instruction *Inst);
> + bool isStoreSinkBarrierInRange(const Instruction& Start,
> + const Instruction& End,
> + AliasAnalysis::Location Loc);
> bool sinkStore(BasicBlock *BB, StoreInst *SinkCand, StoreInst
> *ElseInst);
> bool mergeStores(BasicBlock *BB);
> // The mergeLoad/Store algorithms could have Size0 * Size1 complexity,
> @@ -239,7 +241,7 @@ bool MergedLoadStoreMotion::isLoadHoistB
> const Instruction&
> End,
> LoadInst* LI) {
> AliasAnalysis::Location Loc = AA->getLocation(LI);
> - return AA->canInstructionRangeModify(Start, End, Loc);
> + return AA->canInstructionRangeModRef(Start, End, Loc,
> AliasAnalysis::Mod);
> }
>
> ///
> @@ -389,26 +391,19 @@ bool MergedLoadStoreMotion::mergeLoads(B
> }
>
> ///
> -/// \brief True when instruction is sink barrier for a store
> -///
> -bool MergedLoadStoreMotion::isStoreSinkBarrier(Instruction *Inst) {
> - // FIXME: Conservatively let a load instruction block the store.
> - // Use alias analysis instead.
> - if (isa<LoadInst>(Inst))
> - return true;
> - if (isa<CallInst>(Inst))
> - return true;
> - if (isa<TerminatorInst>(Inst) && !isa<BranchInst>(Inst))
> - return true;
> - // Note: mayHaveSideEffects covers all instructions that could
> - // trigger a change to state. Eg. in-flight stores have to be executed
> - // before ordered loads or fences, calls could invoke functions that
> store
> - // data to memory etc.
> - if (!isa<StoreInst>(Inst) && Inst->mayHaveSideEffects()) {
> - return true;
> - }
> - DEBUG(dbgs() << "No Sink Barrier\n");
> - return false;
> +/// \brief True when instruction is a sink barrier for a store
> +/// located in Loc
> +///
> +/// Whenever an instruction could possibly read or modify the
> +/// value being stored or protect against the store from
> +/// happening it is considered a sink barrier.
> +///
> +
> +bool MergedLoadStoreMotion::isStoreSinkBarrierInRange(const Instruction&
> Start,
> + const Instruction&
> End,
> +
> AliasAnalysis::Location
> + Loc) {
> + return AA->canInstructionRangeModRef(Start, End, Loc,
> AliasAnalysis::Ref);
> }
>
> ///
> @@ -416,27 +411,28 @@ bool MergedLoadStoreMotion::isStoreSinkB
> ///
> /// \return The store in \p when it is safe to sink. Otherwise return
> Null.
> ///
> -StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB,
> - StoreInst *SI) {
> - StoreInst *I = 0;
> - DEBUG(dbgs() << "can Sink? : "; SI->dump(); dbgs() << "\n");
> - for (BasicBlock::reverse_iterator RBI = BB->rbegin(), RBE = BB->rend();
> +StoreInst *MergedLoadStoreMotion::canSinkFromBlock(BasicBlock *BB1,
> + StoreInst *Store0) {
> + DEBUG(dbgs() << "can Sink? : "; Store0->dump(); dbgs() << "\n");
> + for (BasicBlock::reverse_iterator RBI = BB1->rbegin(), RBE =
> BB1->rend();
> RBI != RBE; ++RBI) {
> Instruction *Inst = &*RBI;
>
> - // Only move loads if they are used in the block.
> - if (isStoreSinkBarrier(Inst))
> - break;
> - if (isa<StoreInst>(Inst)) {
> - AliasAnalysis::Location LocSI = AA->getLocation(SI);
> - AliasAnalysis::Location LocInst = AA->getLocation((StoreInst
> *)Inst);
> - if (AA->isMustAlias(LocSI, LocInst)) {
> - I = (StoreInst *)Inst;
> - break;
> - }
> + if (!isa<StoreInst>(Inst))
> + continue;
> +
> + StoreInst *Store1 = cast<StoreInst>(Inst);
> + BasicBlock *BB0 = Store0->getParent();
> +
> + AliasAnalysis::Location Loc0 = AA->getLocation(Store0);
> + AliasAnalysis::Location Loc1 = AA->getLocation(Store1);
> + if (AA->isMustAlias(Loc0, Loc1) && Store0->isSameOperationAs(Store1)
> &&
> + !isStoreSinkBarrierInRange(*Store1, BB1->back(), Loc1) &&
> + !isStoreSinkBarrierInRange(*Store0, BB0->back(), Loc0)) {
> + return Store1;
> }
> }
> - return I;
> + return nullptr;
> }
>
> ///
> @@ -548,8 +544,7 @@ bool MergedLoadStoreMotion::mergeStores(
>
> Instruction *I = &*RBI;
> ++RBI;
> - if (isStoreSinkBarrier(I))
> - break;
> +
> // Sink move non-simple (atomic, volatile) stores
> if (!isa<StoreInst>(I))
> continue;
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150217/ec5f11bf/attachment.html>
More information about the llvm-commits
mailing list