[llvm] r180875 - SROA: Generate selects instead of shuffles when blending values because this is the cannonical form.

[Nadav Rotem]

Hi Nick, 

Thanks for letting me know about this. I looked at the test case in r164763 and I can't reproduce the crash on ARM, and the code on X86 looks pretty good (with vblendps). We should generate *perfect* code for vector-selects with constant masks because it is really easy to type-legalize constants. We do have a problem with vector-selects that use masks that are defined in separate basic blocks, because of SelectionDAG limitations.  

Thanks,
Nadav

On May 5, 2013, at 4:14 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> Nadav Rotem wrote:
>>> 
>>> Why?
>> 
>> Hello,
>> 
>>> (not that i disagree, I just don't think you've explained it fully...)
>> 
>> I think that the motivation for this patch is clear in light of the
>> discussion that we had on the mailing list yesterday. The compiler is
>> very conservative about optimizing shuffles and it usually does not
>> modify shuffles. However we do optimize vector selects. Selects are
>> simpler, to understand and optimize and they do not require 1000 lines
>> of c++ code to lower.
> 
> Then you'll want to revert r164763. http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20120924/151805.html
> 
> When I added that patch, I found that the X86 backend generated much better code with the shuffle than the select, and the ARM backend generated correct code with the shuffle but crashed in codegen on the select.
> 
> Nick
> 
>>> Also, why doesn't instcombine canonicalize shuffles to selects if they
>>> are the canonical form?
>> 
>> This is also something that needs to be done, but it is not a part of
>> this patch.
>> 
>>> The whole way I picked a form was to look at what would get
>>> canonicalized if the user wrote it, and match that. Until the rest of
>>> the pipeline is canonicalizing in this way, I worry greatly about this
>>> being a deeply fragile hack.
>> 
>> I don't see why this is a hack. The current design is that we don't
>> touch or generate new shuffles. Modifying to SROA generate selects (that
>> are equivalent to the shuffles that were previously generated) only
>> continues with the current design. Are you suggesting that we start
>> optimizing shuffles at the IR level ?
>> 
>>> Also, as select has *numerous* other problems, it would seem really
>>> important to teach the backend to optimize shuffles that are blends
>>> rather than paper over this deficiency in SROA.
>> 
>> The backends already do a good job lowering shuffle-blends. The problem
>> is with other optimizations, such as SimplifyDemandedBits, that do not
>> touch shuffles, but can handle selects.
>> 
>> Why do you say that this is a deficiency in SROA ?
>> 
>>> Why not call this a select? that's what it is.
>> 
>> You are blending two values to create a single vector. I think that the
>> name blend reflects the operation, and not the actual instruction that
>> we generate. Maybe it will be a good idea to change the name 'shuffle'
>> to 'widen' because we widen the data inside the vector.
>> 
>>> 
>>> Here and below you have significantly decreased the specificity of the
>>> check. Can you please preserve that? I really dislike checking
>>> instructions occur but not checking that the values they produce are
>>> used in the correct context. While sometimes it is impossible, here
>>> the code *already did that*. =/
>> 
>> The test has four lines, and we check every single instruction using
>> CHECK-NEXT. I don't think that matching the variable name adds and
>> safety to the test.
>> 
>> Thanks,
>> Nadav
>> 
>> 
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130505/7362f100/attachment.html>