[PATCH] Scalarize select vector arguments when extracted

[Matt Arsenault]

On Sep 10, 2013, at 12:26 , Nadav Rotem <nrotem at apple.com> wrote:

> Hi Matt, 
> 
> I am sorry I missed your first email. Thanks for working on this. 
> 
>> When the elements are extracted from a select, do the select on the extracted scalars from the input.
>> 
> 
> I am not sure that this is always profitable or that it should be done in InstCombine. It is hard to know at IR-level (during inst-combine) if it is better to extract twice and perform a single select or if it is better to perform an vector select and extract once. I think that this kind of decision should be made in SelectionDAG. 

I was following your suggestion from the case I asked about here: http://lists.cs.uiuc.edu/pipermail/llvmdev/2013-August/065033.html

My first thought was to do this for the scalar select on vectors case, which I think would more universally be better. I wasn't sure about the vselect case. Would it make sense to always do only the scalar case here?


> 
>> With this, the vectorizer still isn't able to put a vector select back together. It is skipped because some of the insertelements are before the last select being vectorized.
>> 
> 
> I assume that you mean the SLP-vectorizer.  What is the problem ? Do you have a test case ? I’d be happy to take a look.

My SLP vectorizer patch to look at insertelement sequences handled this when I manually scalarized the incoming vector arguments to the function. This is the second half of trying to fix the whole problem. The difference between the instcombine output with this patch and my manually scalarized version is the order of the instructions. In my neat manual version, all of the same operation are done together at once, but the output of this patch has them interspersed for each component, but that shouldn't really matter

IIRC when I tried to find out why, it was rejected in BoUpSLP::buildTree_rec around line 582 
      int UserIndex = BN.getIndex(User);
      if (UserIndex < MyLastIndex) {

        DEBUG(dbgs() << "SLP: Can't schedule extractelement for "
              << *User << ". \n");
        newTreeEntry(VL, false);
        return;
      }

Attached is the manual one that was the test for my insertelement SLP vectorizer patch, and then the version that isn't vectorized that instcombine produces with this patch.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: scalarized_with_instcombine.ll
Type: application/octet-stream
Size: 1851 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130910/df4ee8a7/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: manual_scalarize.ll
Type: application/octet-stream
Size: 4808 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130910/df4ee8a7/attachment-0001.obj>
-------------- next part --------------

>> Also fixes some missing CHECK-LABELs in the test that changed, but those are separate.
>> 
> 
> Can you commit the label fixes as a separate patch ?

I've committed that part