[PATCH] Fix PR19657 : SLP vectorization doesn't combine scalar load to vector loads

Nadav Rotem nrotem at apple.com
Wed Jun 4 22:37:28 PDT 2014


Hi Karthik, 

I am really happy that you are working on the SLP vectorizer. I am sorry for the delay in response. I am in a conference and don’t have access to my computer most hours of the day. I’ll try to review your other patches soon.  The code changes in this patch look okay, but please document this part of the code. It is really odd and without proper documentation it is impossible to figure out why we are doing this. Also, I may have missed a previous email, but were there any performance changes on the llvm test suite?

Thanks,
Nadav

On Jun 2, 2014, at 5:20 AM, Karthik Bhat <kv.bhat at samsung.com> wrote:

> Hi Nadav, Arnold,
> Clang format code as per review comments. I think we cannot swap the right and left in buildTree_rec as we need to process the larger subtree before the smaller one and in case were Left subtree is larger we need to process it first hence we will need the check.
> 
> Does this look good to commit? I'm also looking into a generic solution for this problem as Raul suggested but it might need more time and benchmarking to make sure the benifit outweighs the overhead.
> 
> Thanks for your time.
> Regards
> Karthik Bhat
> 
> http://reviews.llvm.org/D3800
> 
> Files:
>  lib/Transforms/Vectorize/SLPVectorizer.cpp
>  test/Transforms/SLPVectorizer/X86/pr19657.ll
> <D3800.10013.patch>





More information about the llvm-commits mailing list