<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0cm;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri","sans-serif";}
@page WordSection1
{size:612.0pt 792.0pt;
margin:72.0pt 90.0pt 72.0pt 90.0pt;}
div.WordSection1
{page:WordSection1;}
/* List Definitions */
@list l0
{mso-list-id:799957587;
mso-list-type:hybrid;
mso-list-template-ids:-1214880770 44585902 67698691 67698693 67698689 67698691 67698693 67698689 67698691 67698693;}
@list l0:level1
{mso-level-start-at:0;
mso-level-number-format:bullet;
mso-level-text:-;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Calibri","sans-serif";
mso-fareast-font-family:"Times New Roman";}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Symbol;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Symbol;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
ol
{margin-bottom:0cm;}
ul
{margin-bottom:0cm;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I fixed the bug in 229495.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Thanks.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal" style="margin-left:36.0pt;text-indent:-18.0pt;mso-list:l0 level1 lfo1">
<![if !supportLists]><span style="font-family:"Calibri","sans-serif";color:#31849B"><span style="mso-list:Ignore">-<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span dir="LTR"></span><b><i><span style="color:#31849B"> Elena<o:p></o:p></span></i></b></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> David Majnemer [mailto:david.majnemer@gmail.com]
<br>
<b>Sent:</b> Tuesday, February 17, 2015 10:43<br>
<b>To:</b> Demikhovsky, Elena<br>
<b>Cc:</b> LLVM Commits; Bolshinsky, Ella<br>
<b>Subject:</b> Re: [llvm] r224247 - Sink store based on alias analysis<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">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>.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">On Mon, Dec 15, 2014 at 6:09 AM, Elena Demikhovsky <<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>> wrote:<o:p></o:p></p>
<p class="MsoNormal">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><o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<p>---------------------------------------------------------------------<br>
Intel Israel (74) Limited</p>
<p>This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.</p></body>
</html>