This is an archive of the discontinued LLVM Phabricator instance.

[AVX-512] Add support for lowering shuffles to VALIGND/VALIGNQ
ClosedPublic

Authored by craig.topper on Nov 3 2016, 11:21 PM.

Details

Summary

VALIGND and VALIGNQ are similar to PALIGNR but instead of working on a 128-bit lane they work on the entire vector register. This change leverages the shuffle rotate detection code used for PALIGNR to detect these cases.

Diff Detail

Event Timeline

craig.topper retitled this revision from to [AVX-512] Add support for lowering shuffles to VALIGND/VALIGNQ.
craig.topper updated this object.
craig.topper added reviewers: delena, RKSimon.
craig.topper added subscribers: llvm-commits, Farhana.
RKSimon edited edge metadata.Nov 7 2016, 11:27 AM

Please can you add the new tests to trunk with the existing lowering?

For 128-bit vectors, we favor VALIGND/Q over PALIGNR so that we naturely match the vector element size in case the shuffle is part of a masked operation

Would we be better off putting this logic into combineSelect() and keep to PALIGNR? Detect mask/maskz selects taking a bitcasted target shuffles/logicals/whatever and attempt to narrow/widen them back? Not certain if this would actually work in general, its just a thought.

That sounds like a good idea. We can leave it as PALIGNR and not even try to do PALIGND/Q for 128-bit until combineSelect. Less code in shuffle lowering is probably best.

craig.topper edited edge metadata.

Removing 128-bit support at Simon's suggestion.

Add current codegen to trunk to show the diffs in the tests?

lib/Target/X86/X86ISelLowering.cpp
12215

Will this ever fire?

craig.topper updated this object.

Updating tests to only show difference in codegen with this patch.

lib/Target/X86/X86ISelLowering.cpp
12215

VALIGND/Q work on the whole vector. PALIGNR works on 128-bit lanes so they should cover different cases.

RKSimon accepted this revision.Nov 11 2016, 10:56 AM
RKSimon edited edge metadata.

LGTM - please can you create a PR about the AVX512 combineSelect shuffle/logical widening/narrowing idea (unless you intend to do it very soon).

I'll see if we can add matchVectorShuffleAsRotate to matchBinaryPermuteVectorShuffle.

This revision is now accepted and ready to land.Nov 11 2016, 10:56 AM
This revision was automatically updated to reflect the committed changes.