<div dir="ltr">Thanks, I'll commit the patch shortly.<br></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, May 13, 2013 at 2:52 PM, Nadav Rotem <span dir="ltr"><<a href="mailto:nrotem@apple.com" target="_blank">nrotem@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">The patch LGTM.  The StackColoring patch is still too conservative and I am consulting with Jakob and Andy about possible solutions.<div>
<br><div><div><div class="h5"><div>On May 13, 2013, at 10:33 AM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>> wrote:</div><br></div></div><blockquote type="cite"><div style="letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div><div class="h5"><div dir="ltr">This is the email I sent last week.<br><div><br><div class="gmail_quote">---------- Forwarded message ----------<br>From:<span> </span><b class="gmail_sendername">Akira Hatanaka</b><span> </span><span dir="ltr"><<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>></span><br>
Date: Wed, May 8, 2013 at 7:04 PM<br>Subject: [PATCH] Minor fix to StackColoring to avoid needlessly clearing mem operands.<br>To: LLVM Developers Mailing List <<a href="mailto:llvmdev@cs.uiuc.edu" target="_blank">llvmdev@cs.uiuc.edu</a>><br>
<br><br><div dir="ltr"><div>The following code snippet taken from StackColoring::remapInstructions clears a mem operand if it can't guarantee whether the memoperand's underlying value aliases with the merged allocas:<br>
<br>// Update the MachineMemOperand to use the new alloca.<br> 522       for (MachineInstr::mmo_iterator MM = I->memoperands_begin(),<br>....<br>         // Climb up and find the original alloca.<br> 532         V = GetUnderlyingObject(V);<br>
 533         // If we did not find one, or if the one that we found is not in our<br> 534         // map, then move on.<br> 535         if (!V || !isa<AllocaInst>(V)) {<br> 536           // Clear mem operand since we don't know for sure that it doesn't<br>
 537           // alias a merged alloca.<br> 538           MMO->setValue(0);<br> 539           continue;<br> 540         }<br><br>The attached patch makes the code above less conservative. It avoids clearing a mem operand if its underlying value is a PseudoSourceValue and PseudoSourceValue::isConstant returns true. This enables MachineLICM to hoist loads from GOT out of a loop (see test case in stackcoloring-test.patch).<br>
<br></div><div>Please review.<br><br>Question: Why does it need to clear a mem operand if the underlying object is not an AllocaInst? I am not sure if I understand the reason for this.<br><br></div></div></div><br></div></div>
</div></div><span><stackcoloring.patch></span><span><stackcoloring-test.patch></span>_______________________________________________<div class="im"><br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a><span> </span>        <a href="http://llvm.cs.uiuc.edu/" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></div></div></blockquote></div><br></div></div></blockquote></div><br></div>