[PATCH] [X86 DAG] Lower vselects with a constant condition as blend+imm if possible

[Nadav Rotem]

On May 15, 2014, at 4:57 PM, Filipe Cabecinhas <filcab+llvm.phabricator at gmail.com> wrote:

> Hi Nadav,
> 
> I'm not very familiar with the cost model part, but I couldn't find proper places that I have to change:
> I'm assuming you're talking about getCmpSelInstrCost() in X86TargetTransformInfo, is this right?

Yes.

> 
> If so, the function only seems to get the types of the instruction's condition and result, not the proper arguments.
> If we don't get the arguments, we can't be sure if this will be a cheap select.

Yes. Another problem that we have is that we don’t know if the arguments are constant or not. There is a tradeoff here between having a small implementation that does not duplicate the entire codegen and between the accuracy of the cost model. 

> 
> Even with that, the BaseTargetTransformInfo only cares about nodes set to Expand. All other nodes' cost (Legal, Custom, Promote) are the type legalization cost + 1 (BasicTargetTransformInfo.cpp:445).
> X86TargetTransformInfo.cpp:576 has the X86 specialization, but this one just adds what's on the cost table to the type legalization. I don't think it's really needed to add cost table entries just to, in the end, have the same behaviour as we now have.

Okay, in that case we don’t need to change anything.   

> 
> I also added a test for the costs to lower a vselect, since we had none. But I can commit it without the table changes.

Sounds good. Thank you for working on this and for improving our vselect code generation. 

> What do you think? I have the tables ready to commit anyway, but I don't think we need to.
> 
>  Filipe
> 
> 
> 
> 
> On Wed, May 14, 2014 at 2:49 PM, Nadav Rotem <nrotem at apple.com> wrote:
> Excellent. Please update the cost model as well.
> 
> Thanks :)
> 
> http://reviews.llvm.org/D3752
> 
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140516/3066c8a3/attachment.html>