<div dir="ltr">On Tue, Sep 17, 2013 at 11:23 PM, Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank" class="cremed">sepavloff@gmail.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thank you for sharing thoughts, Sean.<div><br><div>I wonder if any target other than x86 can have similar problem? Should this MI-level peephole pass be x86 specific?</div>
</div></div></blockquote><div><br></div><div>I would start specific and generalize incrementally as we have concrete test cases that benefit from it.</div><div><br></div><div>So, start with an x86-specific pass, specifically targetting patterns of repeated immediate operands that can be replaced with a register operand. This should have nothing to do with 'mov' instructions specifically, instead focusing on immediate operands where encoding a register operand is smaller. There should be a pretty reasonable set of heuristics here that catch your memset case as well as cases like a series of "add -2" instructions that sometimes come up in refcounting code.</div>
<div><br></div><div>We can look at more generalizations once this works well. =]</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>
<br></div><div>Thanks,</div>
<div>--Serge</div></div><div class="gmail_extra"><div><div class="h5"><br><br><div class="gmail_quote">2013/9/18 Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank" class="cremed">silvas@purdue.edu</a>></span><br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div>On Wed, Sep 18, 2013 at 12:17 AM, Alex Rosenberg <span dir="ltr"><<a href="mailto:alexr@leftfield.org" target="_blank" class="cremed">alexr@leftfield.org</a>></span> wrote:<br></div><div class="gmail_extra">
<div class="gmail_quote"><div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="auto"><div>FWIW, Sean ran into this exact issue earlier in the Summer. On x86, these long instructions with 32-bit immediate zeros in them added up to a substantial amount of code. IIRC, he noticed lots of those in global constructors.</div>
<div><br></div><div>Sean?<span><font color="#888888"><br><br></font></span></div></div></blockquote><div><br></div></div><div>The issue I saw this Summer was a regression of our internal branch vs. trunk (i.e., trunk didn't have the code size issue). The issue appeared to be fixed in newer versions of the toolchain repo that had merged up to LLVM 3.3.</div>
<div><br></div><div>I agree with Chandler though that this would best be done as an MI-level peephole pass. Besides the register pressure thing, if optimizing for size, there are some pretty "low-level" code size hazards that are basically due to quirks in the instruction encoding, Off the top of my head I can think of:</div>
<div>* once an offset from a base pointer becomes large enough (doesn't fit in a signed byte), the immediate is encoded as 32-bit instead. Each time you can avoid this you save 3 bytes of code size basically.</div><div>
* Some AVX2 instructions interpret these immediates differently (as multiples of the logical vector element size), which increases the actual range of an imm8 operand, so the simple rule "keep the offset from the base pointer so that it fits in an imm8" isn't quite so simple.</div>
<div>* some registers need larger encodings for the indexing (r8-r15 need 1-byte REX (but that may already be required for a 64-bit operation); rsp+imm8 needs a SIB, since the "usual" encoding for that is special-cased for use for RIP-rel addressing). These usually will save you only 1 byte.</div>
<span><font color="#888888">
<div><br></div><div>-- Sean Silva</div></font></span><div><div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div dir="auto"><div><span><font color="#888888">Alex</font></span></div><div><div><div><br>On Sep 16, 2013, at 9:20 PM, Nadav Rotem <<a href="mailto:nrotem@apple.com" target="_blank" class="cremed">nrotem@apple.com</a>> wrote:<br>
<br></div><blockquote type="cite"><div><div>Hi, </div><div><br></div><div>I am sorry for missing this patch the last time you sent it. Do you have any performance numbers that show the profitability of this transformation ? I also feel like SelectionDAG is not the right place to implement this kind of optimization. I think that introducing virtual registers would inhibit other optimizations on the DAG. I also agree that this optimization can be generalized to any sequence of machine instructions that repeatedly use the same constant. </div>
<div><br></div><div>I have a few comments on the patch itself.</div><div><br></div><div>You can commit the documentation of this method even without the change.</div><div><br></div><div><div>+/// \brief Lower the call to 'memset' intrinsic function into a series of store</div>
<div>+/// operations.</div><div>+///</div><div>+/// \param DAG Selection DAG where lowered code is placed.</div><div>+/// \param dl Link to corresponding IR location.</div><div><br></div><div>...</div><div><br></div><div>
+/// The function tries to replace 'llvm.memset' intrinsic with several store</div><div>+/// operations and value calculation code. This is usually profitable for small</div><div>+/// memory size.</div><div> static SDValue getMemsetStores(SelectionDAG &DAG, SDLoc dl,</div>
<div> SDValue Chain, SDValue Dst,</div><div> SDValue Src, uint64_t Size,</div></div><div><br></div><div><br></div><div>The code below is a little confusing. Can you please add braces to the else clause, or at least separate the next statement with a line break ?</div>
<div><br></div><div><br></div><div><div>+ } else</div><div>+ LongestValueInReg = MemSetValue;</div><div>+ SDValue RegInitialization = Chain;</div><div>+</div><div> </div></div><div><br></div><div>Also here. Line break after the first "Value = “ line. I would also add braces around the last “Value = “ line.</div>
<div><br></div><div><div>+ TLI.isTruncateFree(LargestVT, VT)) {</div><div>+ if (ValueReg != 0)</div><div>+ Value = DAG.getCopyFromReg(RegInitialization, dl, ValueReg, LargestVT);</div><div>+ Value = DAG.getNode(ISD::TRUNCATE, dl, VT, Value);</div>
<div>+ }</div><div> else</div><div> Value = getMemsetValue(Src, VT, DAG, dl);</div><div> }</div></div><div><br></div><div><br></div><div>Thanks,</div><div>Nadav</div><div><br></div><br><div><div>On Sep 16, 2013, at 2:55 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank" class="cremed">chandlerc@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div dir="ltr">I wonder whether this is the best approach.<div><br></div><div>Specifically, this is trading off register pressure for code size. I think it is probably the correct call in many cases, but perhaps not in all cases. Also, it has nothing to do with memset, and everything to do with any sequence of immediate operand movs.</div>
<div><br></div><div>I think this would be done somewhat better as an MI peephole that takes into account register pressure and how many mov instructions share the immediate to tradeoff the register pressure and potential instructions to materialize the constant into a register against the shrink of immediate operand movs.</div>
<div><br></div><div>But yes, Nadav or Andy would have more insights here...</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Sep 16, 2013 at 10:00 AM, Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank" class="cremed">sepavloff@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">Friendly ping.</div><div class="gmail_extra">
<div><div><br><br><div class="gmail_quote">2013/9/10 Serge Pavlov <span dir="ltr"><<a href="mailto:sepavloff@gmail.com" target="_blank" class="cremed">sepavloff@gmail.com</a>></span><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"> Cosmetical update to the patch<br>
If block size is small, the size of largest move chunk may be smaller than the<br>
natural register width and target may not have resisters of such width. Example<br>
is 32-bit target that does not have 16-bit registers. Avoid allocation such<br>
illegal registers.<br>
<br>
Hi bkramer, nadav,<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1484" target="_blank" class="cremed">http://llvm-reviews.chandlerc.com/D1484</a><br>
<br>
CHANGE SINCE LAST DIFF<br>
<a href="http://llvm-reviews.chandlerc.com/D1484?vs=3692&id=4161#toc" target="_blank" class="cremed">http://llvm-reviews.chandlerc.com/D1484?vs=3692&id=4161#toc</a><br>
<div><br>
Files:<br>
include/llvm/Target/TargetLowering.h<br>
lib/CodeGen/SelectionDAG/SelectionDAG.cpp<br>
lib/Target/X86/X86ISelLowering.cpp<br>
lib/Target/X86/X86ISelLowering.h<br>
test/CodeGen/X86/memset-sse-stack-realignment.ll<br>
test/CodeGen/X86/memset.ll<br>
test/CodeGen/X86/tlv-1.ll<br>
</div></blockquote></div><br><br clear="all"><div><br></div></div></div><span><font color="#888888">-- <br>Thanks,<br>--Serge<br>
</font></span></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>
</blockquote></div><br></div></blockquote><blockquote type="cite"><div><span>_______________________________________________</span><br><span>llvm-commits mailing list</span><br><span><a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="cremed">llvm-commits@cs.uiuc.edu</a></span><br>
<span><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></span><br></div></blockquote></div></div></div></blockquote>
</div></div></div>
<br></div>
</div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br><br clear="all"><div><br></div></div></div><span class="HOEnZb"><font color="#888888">-- <br>Thanks,<br>--Serge<br>
</font></span></div>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>