<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On May 6, 2014, at 12:17 PM, Philip Reames <<a href="mailto:listmail@philipreames.com">listmail@philipreames.com</a>> wrote:</div><div><br></div><div>Hi Philip,</div><div><br></div><br class="Apple-interchange-newline"><blockquote type="cite">
<meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type">
<div bgcolor="#FFFFFF" text="#000000">
Nadav,<br>
<br>
Could you spell out what you believe to be the right approach w.r.t.
this topic? I'd also appreciate any links to previous discussion on
this topic; a quick google search didn't find much. <br></div></blockquote><div><br></div><div>This is one thread:</div><div><a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120402/140452.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120402/140452.html</a></div><div><br></div><div>This is another:</div><div><a href="http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130429/173217.html">http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20130429/173217.html</a></div><div><br></div><div>A presentation that discusses how difficult it is to lower shuffles: <a href="http://www.nondot.org/~sabre//2012-04-02-CGOKeynote.pdf">http://www.nondot.org/~sabre//2012-04-02-CGOKeynote.pdf</a></div><div><br></div><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
Reading through Filipe's patch, it seems entirely reasonable to me.
It's a strict reduction in the number of shufflevectors in the IR. </div></blockquote><div><br></div><div>The problem is not the number of shuffles in the IR. The problem is the number of generated instructions. Shuffle instructions are really difficult to lower and the current design decision is not to introduce new shuffle instructions. </div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
Even given the fact we can't lower shuffle vector perfectly in all
cases, I don't see the harm in this patch. I'm not trying to say
you're wrong; I'm merely trying to understand what it is I'm not
seeing here. <br></div></blockquote><div><br></div><div>There is lots of code out there that uses llvm shuffles with knowledge of the underlying architecture. This is very common in OpenCL and OpenGL implementation and introducing new shuffles that we can’t lower properly can break people’s hand-optimized matrix multiplication code.</div><div><br></div><div>Thanks,</div><div>Nadav </div><br><blockquote type="cite"><div bgcolor="#FFFFFF" text="#000000">
<br>
Philip<br>
<br>
<br>
<div class="moz-cite-prefix">On 05/06/2014 09:22 AM, Nadav Rotem
wrote:<br>
</div>
<blockquote cite="mid:30BC393E-B193-4EA4-8664-FF3785FB7D08@apple.com" type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=ISO-8859-1">
<div>Hi Filipe, </div>
<div><br>
</div>
<div>Thank you for working on this. I appreciate the fact that you
are eager to improve LLVM and that you went through the trouble
of testing your code on multiple targets and on considering
different alternatives to the problem. I think that the patch
below is unrelated to the blend->select lowering so I will
only address this patch. We discussed on the possibility of
creating new shuffles on the llvm mailing lists more than once.
We decided not to create new shuffles because we may create new
shuffle patterns that we can’t lower efficiently. I ran into
this problem a few times myself in different LLVM-based
projects, mainly in the domain of graphics where people write
swizzles themselves. Please do not commit this patch because it
creates new shuffles. It is not the correct approach. You may
get lucky in you selection of testcase, but I am sure that there
are lots of other shuffle inputs on X86 and other targets that
this patch makes worse. </div>
<div><br>
</div>
<div>Thanks,</div>
<div>Nadav </div>
<div><br>
</div>
<div><br>
</div>
<div>
<blockquote type="cite">
<div dir="ltr">Hi Nadav,
<div><br>
</div>
<div>I'd really like this patch to get in, since it improves
code generation on many targets (at the very least, I've
tried arm and arm64 (mcpu=cyclone), ppc32 and ppc64
(mcpu=g5), and x86 and x86-64 (mcpu=nehalem) and they all
improve (they end up with no loads vs. 1 or 2 loads, and
code size is about half of what it was).</div>
<div><br>
</div>
<div>Even if we'd like to make the select instruction be the
proper canonicalization of this operation, it seems that,
for now, a shufflevector is better when it comes to
lowering it.</div>
<div><br>
</div>
<div>Several targets have the action for the vselect as
Expand, which will generate a bunch of code which would
then have to be pattern-matched, instead of lowering the
vselect as custom and falling-through to Expand if there's
no special-case instruction/set of instructions that we
can emit.</div>
<div><br>
</div>
<div>I ran opt+llc in this way, for the variants I mentioned
above (./bin/opt is opt with this optimization):</div>
<div>./bin/opt -O3 select-from-shuffles.ll | ./bin/llc
-march=arm64 -mcpu=cyclone > arm64-cylone-opt<br>
</div>
<div>opt -O3 select-from-shuffles.ll | ./bin/llc
-march=arm64 -mcpu=cyclone > arm64-cylone-opt<br>
</div>
<div><br>
</div>
<div>I can understand if we eventually drop this after we
make the backend treat the select as the canonical form of
this operation and have llvm always use that form, but for
now it seems that the shufflevector is better lowered in
some important targets.</div>
<div><br>
</div>
<div><br>
</div>
<div> Filipe</div>
<div><br>
</div>
<div><br>
</div>
</div>
<div class="gmail_extra"><br>
<br>
<div class="gmail_quote">On Fri, May 2, 2014 at 11:55 PM,
Nadav Rotem <span dir="ltr"><<a moz-do-not-send="true" 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">Hi
Filipe,<br>
<br>
We don’t generate new shufflevector instructions during
optimizations because the lowering of shuffle
instructions is really complicated and we don’t want to
generate a shuffle that we don’t lower by accident.<br>
<br>
Thanks,<br>
Nadav<br>
<div>
<div class="h5"><br>
On May 2, 2014, at 4:34 PM, Filipe Cabecinhas <<a moz-do-not-send="true" href="mailto:filcab%2Bllvm.phabricator@gmail.com">filcab+llvm.phabricator@gmail.com</a>>
wrote:<br>
<br>
> Remove unused argument<br>
><br>
> <a moz-do-not-send="true" href="http://reviews.llvm.org/D3561" target="_blank">http://reviews.llvm.org/D3561</a><br>
><br>
> Files:<br>
>
lib/Transforms/InstCombine/InstCombineSelect.cpp<br>
> test/Transforms/InstCombine/select.ll<br>
</div>
</div>
>
<D3561.9050.patch>_______________________________________________<br>
> llvm-commits mailing list<br>
> <a moz-do-not-send="true" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a moz-do-not-send="true" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
</blockquote>
</div>
<br>
</div>
</blockquote>
</div>
<br>
<br>
<fieldset class="mimeAttachmentHeader"></fieldset>
<br>
<pre wrap="">_______________________________________________
llvm-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</div>
</blockquote></div><br></body></html>