This is an archive of the discontinued LLVM Phabricator instance.

X86: Fix LowerBUILD_VECTORAsVariablePermute for case Src is smaller than Indices
ClosedPublic

Authored by zvi on Jan 9 2018, 7:53 AM.

Event Timeline

zvi created this revision.Jan 9 2018, 7:53 AM

Please can you add a similar test for a bigger source vector, something like: <4 x i32> @foo(<8 x i32> %v, <2 x i32> %indices) ?

zvi added a comment.Jan 9 2018, 10:04 AM

Sure, but looking at your example the return type should have the same number of elements as the indices vector, right?

zvi updated this revision to Diff 129121.Jan 9 2018, 10:30 AM

Added test with source vector larger than indices vector

In D41865#971307, @zvi wrote:

Sure, but looking at your example the return type should have the same number of elements as the indices vector, right?

Yup, sorry for the typo. Are you intending to support cases like this?

zvi added a comment.Jan 10 2018, 4:20 AM
In D41865#971307, @zvi wrote:

Sure, but looking at your example the return type should have the same number of elements as the indices vector, right?

Yup, sorry for the typo. Are you intending to support cases like this?

Will need to think about how/if it can be done. AVX512 masking could make it easier.
Right now i would like to prioritize fixing broken functionality...

RKSimon added inline comments.Jan 10 2018, 8:24 AM
lib/Target/X86/X86ISelLowering.cpp
7867

I don't think you should use IndicesVT - (what happens with <8 x float> @pr35820_alt(<4 x float> %v, <8 x i32> %indices) ?

zvi updated this revision to Diff 129287.Jan 10 2018, 9:25 AM

Fix issue identified by Simon: use original vector type for the insert_vector

zvi marked an inline comment as done.Jan 10 2018, 9:28 AM
RKSimon accepted this revision.Jan 10 2018, 10:19 AM

LGTM - thanks

This revision is now accepted and ready to land.Jan 10 2018, 10:19 AM
This revision was automatically updated to reflect the committed changes.