[llvm] r224247 - Sink store based on alias analysis
Demikhovsky, Elena
elena.demikhovsky at intel.com
Tue Feb 17 05:13:10 PST 2015
I fixed the bug in 229495.
Thanks.
- Elena
From: David Majnemer [mailto:david.majnemer at gmail.com]
Sent: Tuesday, February 17, 2015 10:43
To: Demikhovsky, Elena
Cc: LLVM Commits; Bolshinsky, Ella
Subject: Re: [llvm] r224247 - Sink store based on alias analysis
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<mailto: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<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
---------------------------------------------------------------------
Intel Israel (74) Limited
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150217/91aa77f6/attachment.html>
More information about the llvm-commits
mailing list