This is an archive of the discontinued LLVM Phabricator instance.

[X86] Allow cross-lane permutations for sub targets supporting AVX2.
ClosedPublic

Authored by jbhateja on Sep 1 2017, 11:27 AM.

Details

Summary

Most instructions in AVX work “in-lane”, that is, each source element is applied only to other
elements of the same lane, thus a cross lane permutation is costly and needs more than one instrution.
AVX2 includes instructions to perform any-to-any permutation of words over a 256-bit register
and vectorized table lookup.

This should also Fix PR34369

Diff Detail

Repository
rL LLVM

Event Timeline

jbhateja created this revision.Sep 1 2017, 11:27 AM

Ping reviewers.

guyblank edited edge metadata.Sep 3 2017, 1:06 AM

Hi Jatin,
Thanks for working on this.
You didn't add llvm-commits as a subscriber upon review creation, so this won't show up on the mailing list. Can you open a new review?

Hi Jatin,
Thanks for working on this.
You didn't add llvm-commits as a subscriber upon review creation, so this won't show up on the mailing list. Can you open a new review?

Hi Guy,

llvm-commits is already on subscriber's list for this differential revision. I did not not get your point in opeining a new review.

Thanks

Hi Jatin,
Thanks for working on this.
You didn't add llvm-commits as a subscriber upon review creation, so this won't show up on the mailing list. Can you open a new review?

Hi Guy,

llvm-commits is already on subscriber's list for this differential revision. I did not not get your point in opeining a new review.

Thanks

If you want the llvm-commits mailing list to be notified when you create a review, you need to add it as a subscriber when creating the review, not afterwards.
When you add it after the initial creation, there will be no mail notification to the list.
So in your case, the first message the list got was the 'ping'.
If llvm-commits was added upon creation, the first message would have been:
jatin created this revision.
<the description of the revision>

Which I think is useful for people browsing the list.

Oh Ok , thanks I shall take care in future.

@reviewers , kindly pour in your comment.

RKSimon edited edge metadata.Sep 4 2017, 4:30 AM

I've added the test case for PR34369 at rL312481 - please can you rebase?

lib/Target/X86/X86ISelLowering.cpp
12079 ↗(On Diff #113560)

Update this comment to explain the AVX2 path

jbhateja updated this revision to Diff 113782.Sep 4 2017, 11:27 AM
jbhateja updated this revision to Diff 113783.Sep 4 2017, 11:34 AM
  • Reverting lit script change.
aymanmus edited edge metadata.Sep 5 2017, 1:16 AM

The patch shows impressive results.
LGTM, unless @RKSimon have other comments.

RKSimon accepted this revision.Sep 5 2017, 4:15 AM

LGTM with a few minors to fix before commit

lib/Target/X86/X86ISelLowering.cpp
12071 ↗(On Diff #113783)

clang-format ? (80-column limit)

12079 ↗(On Diff #113783)

half of the lanes are not

This revision is now accepted and ready to land.Sep 5 2017, 4:15 AM
jbhateja updated this revision to Diff 113949.Sep 5 2017, 7:53 PM
  • Formatting changes
This revision was automatically updated to reflect the committed changes.