[PATCH] [x86] Simplify vector selection

[Adam Nemet]

On Sep 30, 2014, at 11:04 AM, Chandler Carruth <chandlerc at google.com> wrote:

> This patch doesn't really make sense to me...
> 
> On Tue, Sep 30, 2014 at 10:26 AM, Robert Khasanov <rob.khasanov at gmail.com> wrote:
> [x86] Simplify vector selection if condition value type matches vselect value type and true value is all ones or false value is all zeros.
> This transformation worked if selector is produced by SETCC, however SETCC is needed only if we consider to swap operands. So I replaced SETCC check for this case.
> 
> What is your actual goal here?

The goal was to separate the lowering of the CMPM intrinsics and a related canonicalization/simplification.   This is the simplification:

(vselect VK8:$mask, (PCMPEQM %a, %b), (v8i1 (BUILD_VECTOR 0, 0, 0, 0, 0, 0, 0, 0)))

to

(and VK8:$mask, (PCMPEQM %a, %b))

Right now in the intrinsic lowering code we special-case CMPM and lower it to an AND expression rather than the generic VSELECT.  My comment on a previous patch was to avoid special-casing like this and letting the DAGCombiner do its job.  This is what the patch is trying to do.


> Also I removed special case for cmp instructions in getVectorMaskingNode. Now cmp intrinsics lower as other intrinsics through VSELECT, and then VSELECT tranforms to AND in PerformSELECTCombine.
> 
> This should be a separate patch.
> 
> New tests are not added, existed intrinsics tests on vpcmpeq{bwdq} cover these cases.
> 
> I don't get it.
> 
> Does this patch have no functional impact? If so, say so, and justify both why it should have no functional impact and why the refactoring is desirable? The change looks like it *should* have functional impact…

It shouldn’t have functional impact, it just stages the lowering differently:

intrinsic -> AND   // all done by intrinsic lowering

intrinsic -> VSELECT -> AND  // first is done by lowering and the second by the DAGCombiner

And this is why the old test which ensures that at the end we do generate the cmpm can cover this patch.

Adam

> Also, you did see the recent churn on VSELECT? I'm planning to implement the new plan I outlined in my commit log. It would be great for you to review those patches and the plan I outlined and comment on it (I asked for comments).

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140930/98a6cf52/attachment.html>