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