[PATCH] InstCombine: scalarize vector phis

[Shemer, Anat]

Hi Nadav,

Thanks for your feedback.

>> Thanks for working on this.  Can you split the two changes into two patches ?

Sure.

>> Scalarizing PHIs makes sense to me. Does it solve PR11775 ?

No. the code sample in 11775 has a phi that doesn't increment itself directly, as expected by the patch that I provide. It uses some more complex pattern which increments the vector phi indirectly.

>> Regarding the second change, there is a cost model problem.  The existing code converts a vector cast into a scalar cast, which should be profitable for most platforms. However, your transformation creates an additional cast. Why is it better ?

I'm not sure that I follow. The proposed transformation does the same, it creates only one cast. You can see this in the provided test.

>> I did not read the code but from a quick peek I noticed that the docs are not capitalized and punctuated properly and the variable names are funny, and you left some code commented out and there are extra braces, etc.

I made some corrections.

I'm looking forward to get more feedback.

Thanks, Anat


From: Nadav Rotem [mailto:nrotem at apple.com]
Sent: Tuesday, April 16, 2013 23:48
To: Shemer, Anat
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [PATCH] InstCombine: scalarize vector phis

Hi Anat,

Thanks for working on this.  Can you split the two changes into two patches ?

Scalarizing PHIs makes sense to me. Does it solve PR11775 ?

Regarding the second change, there is a cost model problem.  The existing code converts a vector cast into a scalar cast, which should be profitable for most platforms. However, your transformation creates an additional cast. Why is it better ?

I did not read the code but from a quick peek I noticed that the docs are not capitalized and punctuated properly and the variable names are funny, and you left some code commented out and there are extra braces, etc.

Thanks,
Nadav

On Apr 16, 2013, at 1:11 PM, "Shemer, Anat" <anat.shemer at intel.com<mailto:anat.shemer at intel.com>> wrote:


Hi,

I would like to add to InstCombineVectorOps.cpp a function that sclarizes a vector phi instruction if its only use is an extract operation of one element at a constant location. Such vector phi might be created by the vectorization phase and the generated code is non-optimal.
In addition, in the function InstCombiner::visitExtractElementInst I removed the limitation that extract is promoted over a cast only if the cast has only one use. To make this work I replaced the use of Builder->CreateExtractElement() call with InsertNewInstWith(ExtractElementInst::Create()..), which also inserts the new instruction into the worklist.

I also added a test for these 2 cases.

I will appreciate your feedback.

Thanks, Anat




---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
<InstCombineVectorPHI.diff>_______________________________________________
llvm-commits mailing list
llvm-commits at cs.uiuc.edu<mailto:llvm-commits at cs.uiuc.edu>
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130417/0640cd44/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: InstCombineVectorPHI1.diff
Type: application/octet-stream
Size: 5881 bytes
Desc: InstCombineVectorPHI1.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130417/0640cd44/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: extractNuses.diff
Type: application/octet-stream
Size: 1686 bytes
Desc: extractNuses.diff
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130417/0640cd44/attachment-0001.obj>