[PATCH] Fix for previously reverted SLPVectorizer cost model improvement

[Tyler Nowicki]

Thanks for the review

Here is an updated patch. I made the changes you asked for including splitting out the unrelated code, modifying the comment, removing the lifetime intrinsics. I also adding ctpop which was previously missing.

Tyler

-------------- next part --------------
A non-text attachment was scrubbed...
Name: slp_trivially_vectorizable_intrinsic_svn.patch
Type: application/octet-stream
Size: 5529 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140404/4425104c/attachment.obj>
-------------- next part --------------


On Apr 3, 2014, at 4:30 PM, arnold <aschwaighofer at apple.com> wrote:

> --- include/llvm/IR/IntrinsicInst.h	(revision 205571)
> +++ include/llvm/IR/IntrinsicInst.h	(working copy)
> @@ -44,6 +44,38 @@
>       return (Intrinsic::ID)getCalledFunction()->getIntrinsicID();
>     }
> 
> +    /// Method returns true if any intrinsic argument types are scalar
> +    /// in both vector and scalar forms of the intrisinc.
> 
> The other way round (except for lifetime_start). Like you describe below in the explanation of the patch.
> 
> +    ///
> +    static inline bool isTriviallyVectorizable(Intrinsic::ID ID) {
> +      switch (ID) {
> +        case Intrinsic::sort:
> ...
> +        case Intrinsic::lifetime_start:
> +        case Intrinsic::lifetime_end:
> +          return true;
> +        default:
> +          return false;
> +      }
> +    }
> 
> Because of Intrinsic::lifetime_start the comment is not totally true. A lifetime_start/end is vectorizable because we simple scalarize it in the loop vectorizer. For all other intrinsics we can vectorize them because all their arguments become vectors when vectorized (like you mention below). I think lifetime_start/stop should be handled separately (not be checked as part of isTriviallyVectorizable).
> 
> Can you separate the patch into two patches? One that corrects the handling of mixed argument intrinsic and the second one reverting my revert.
> 
> You can test the first patch by moving both insertelement instructions in the example non-vectorizable-intrinsic.ll before the return.
> 
> Thanks for working on this!
> 
> On Apr 3, 2014, at 2:44 PM, Tyler Nowicki <tnowicki at apple.com> wrote:
> 
>> Hi,
>> 
>> This patch modifies r205260 which was previously reverted by r205018 because it a crash. The original patch by Arch Robison and Arnold Schwaighofer.
>> 
>> This patch stops the SLP vectorizer from trying to vectorize a call that is not trivially vectorizable. An intrinsic is not trivially vectorized if any of the arguments cannot be simply packed together into a tuple. For example, the call to CTLZ cannot be trivially replaced with a vector intrinsic because the second parameter is scalar in both its scalar and vector forms.
>> 
>> Tyler Nowicki
>> 
>> <slp_insertelements_fix2.patch>_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>