<div style="font-family:arial,helvetica,sans-serif;font-size:10pt">On 28 November 2012 16:00, Nadav Rotem <span dir="ltr"><<a href="mailto:nrotem@apple.com" target="_blank">nrotem@apple.com</a>></span> wrote:<br>
<div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: nadav<br>
Date: Wed Nov 28 18:00:08 2012<br>
New Revision: 168832<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=168832&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=168832&view=rev</a><br>
Log:<br>
When combining consecutive stores allow loads in between the stores, if the loads do not alias.<br>
<br>
<br>
Added:<br>
llvm/trunk/test/CodeGen/X86/2012-11-28-merge-store-alias.ll<br>
Modified:<br>
llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
<br>
Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=168832&r1=168831&r2=168832&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=168832&r1=168831&r2=168832&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Wed Nov 28 18:00:08 2012<br>
@@ -291,6 +291,10 @@<br>
unsigned SrcValueAlign2,<br>
const MDNode *TBAAInfo2) const;<br>
<br>
+ /// isAlias - Return true if there is any possibility that the two addresses<br>
+ /// overlap.<br>
+ bool isAlias(LSBaseSDNode *Op0, LSBaseSDNode *Op1);<br>
+<br>
/// FindAliasInfo - Extracts the relevant alias information from the memory<br>
/// node. Returns true if the operand was a load.<br>
bool FindAliasInfo(SDNode *N,<br>
@@ -7552,7 +7556,14 @@<br>
if (BasePtr.first.getOpcode() == ISD::UNDEF)<br>
return false;<br>
<br>
+ // Save the LoadSDNodes that we find in the chain.<br>
+ // We need to make sure that these nodes do not interfere with<br>
+ // any of the store nodes.<br>
+ SmallVector<LSBaseSDNode*, 8> AliasLoadNodes;<br>
+<br>
+ // Save the StoreSDNodes that we find in the chain.<br>
SmallVector<MemOpLink, 8> StoreNodes;<br>
+<br>
// Walk up the chain and look for nodes with offsets from the same<br>
// base pointer. Stop when reaching an instruction with a different kind<br>
// or instruction which has a different base pointer.<br>
@@ -7596,8 +7607,26 @@<br>
// We found a potential memory operand to merge.<br>
StoreNodes.push_back(MemOpLink(Index, Ptr.second, Seq++));<br>
<br>
- // Move up the chain to the next memory operation.<br>
- Index = dyn_cast<StoreSDNode>(Index->getChain().getNode());<br>
+ // Find the next memory operand in the chain. If the next operand in the<br>
+ // chain is a store then move up and continue the scan with the next<br>
+ // memory operand. If the next operand is a load save it and use alias<br>
+ // information to check if it interferes with anything.<br>
+ SDNode *NextInChain = Index->getChain().getNode();<br>
+ while (1) {<br>
+ if (isa<StoreSDNode>(NextInChain)) {<br>
+ // We found a store node. Use it for the next iteration.<br>
+ Index = cast<StoreSDNode>(NextInChain);<br>
+ break;<br>
+ } else if (LoadSDNode *Ldn = dyn_cast<LoadSDNode>(NextInChain)) {<br>
+ // Save the load node for later. Continue the scan.<br>
+ AliasLoadNodes.push_back(Ldn);<br>
+ NextInChain = Ldn->getChain().getNode();<br>
+ continue;<br>
+ } else {<br>
+ Index = NULL;<br>
+ break;<br>
+ }<br>
+ }<br></blockquote><div><br></div><div>This loop is more simply written:</div><div><br></div><div> while (1) {</div><div> if (LoadSDNode *Ldn = dyn_cast<LoadSDNode>(NextInChain)) {</div><div> // Save the load node for later. Continue the scan.<br>
AliasLoadNodes.push_back(Ldn);<br> NextInChain = Ldn->getChain().getNode();<br></div><div> } else {</div><div> // If we found a store node, use it for the next iteration.</div><div> Index = dyn_cast<StoreSDNode>(NextInChain);</div>
<div> break;</div><div> }</div><div> }</div><div><br></div><div>?</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br>
<br>
// Check if there is anything to merge.<br>
@@ -7612,11 +7641,23 @@<br>
// store memory address.<br>
unsigned LastConsecutiveStore = 0;<br>
int64_t StartAddress = StoreNodes[0].OffsetFromBase;<br>
- for (unsigned i=1; i<StoreNodes.size(); ++i) {<br>
+ for (unsigned i = 1, e = StoreNodes.size(); i < e; ++i) {<br>
int64_t CurrAddress = StoreNodes[i].OffsetFromBase;<br>
if (CurrAddress - StartAddress != (ElementSizeBytes * i))<br>
break;<br>
<br>
+ bool Alias = false;<br>
+ // Check if this store interferes with any of the loads that we found.<br>
+ for (unsigned ld = 0, lde = AliasLoadNodes.size(); ld < lde; ++ld)<br>
+ if (isAlias(AliasLoadNodes[ld], StoreNodes[i].MemNode)) {<br>
+ Alias = true;<br>
+ break;<br>
+ }<br>
+<br>
+ // We found a load that alias with this store. Stop the sequence.<br>
+ if (Alias)<br>
+ break;<br>
+<br>
// Mark this node as useful.<br>
LastConsecutiveStore = i;<br>
}<br>
@@ -9680,6 +9721,23 @@<br>
return true;<br>
}<br>
<br>
+bool DAGCombiner::isAlias(LSBaseSDNode *Op0, LSBaseSDNode *Op1) {<br>
+ SDValue Ptr0, Ptr1;<br>
+ int64_t Size0, Size1;<br>
+ const Value *SrcValue0, *SrcValue1;<br>
+ int SrcValueOffset0, SrcValueOffset1;<br>
+ unsigned SrcValueAlign0, SrcValueAlign1;<br>
+ const MDNode *SrcTBAAInfo0, *SrcTBAAInfo1;<br>
+ FindAliasInfo(Op0, Ptr0, Size0, SrcValue0, SrcValueOffset0,<br>
+ SrcValueAlign0, SrcTBAAInfo0);<br>
+ FindAliasInfo(Op1, Ptr1, Size1, SrcValue1, SrcValueOffset1,<br>
+ SrcValueAlign1, SrcTBAAInfo1);<br>
+ return isAlias(Ptr0, Size0, SrcValue0, SrcValueOffset0,<br>
+ SrcValueAlign0, SrcTBAAInfo0,<br>
+ Ptr1, Size1, SrcValue1, SrcValueOffset1,<br>
+ SrcValueAlign1, SrcTBAAInfo1);<br></blockquote><div><br></div><div>Wrong indent on the lines hanging after isAlias( .</div><div><br></div><div>Nick</div><div><br></div></div><br></div>