<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">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>On May 13, 2013, at 10:33 AM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com">ahatanak@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div dir="ltr">This is the email I sent last week.<br><div><br><div class="gmail_quote">---------- Forwarded message ----------<br>From:<span class="Apple-converted-space"> </span><b class="gmail_sendername">Akira Hatanaka</b><span class="Apple-converted-space"> </span><span dir="ltr"><<a href="mailto:ahatanak@gmail.com">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">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><span><stackcoloring.patch></span><span><stackcoloring-test.patch></span>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a><span class="Apple-converted-space"> </span> <a href="http://llvm.cs.uiuc.edu/">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></div></blockquote></div><br></div></body></html>