[PATCH] D103274: [X86] Split FeatureFastVariableShuffle tuning into Lane-Crossing and Per-Lane variants

Sun May 30 03:28:02 PDT 2021

lebedev.ri added a comment.

In D103274#2788536 <https://reviews.llvm.org/D103274#2788536>, @pengfei wrote:

> In D103274#2788519 <https://reviews.llvm.org/D103274#2788519>, @RKSimon wrote:
>
>> I'm happy with this but I do think we should always enable variable shuffles (both types) on all AVX512 targets as well (and maybe land that first to simplify this patch) - @pengfei @craig.topper what do you think?

I'm not sure how that would simplify this patch?

> The patch seems NFC for the existing targets to me, though I don't have a deep cognition on the shuffles' cost. No objections from me.

================
Comment at: llvm/lib/Target/X86/X86.td:326
 // using a variable mask over multiple fixed shuffles.
-def FeatureFastVariableShuffle
-    : SubtargetFeature<"fast-variable-shuffle",
-                       "HasFastVariableShuffle",
-                       "true", "Shuffles with variable masks are fast">;
+def FeatureFastVariableCrossLaneShuffle
+    : SubtargetFeature<"fast-variable-crosslane-shuffle",
----------------
pengfei wrote:
> Nit, are pre-lane shuffles always fast if the target has fast cross-lane shuffles? In this mean, we can keep `FeatureFastVariableShuffle` and make it implicate `FeatureFastVariablePerLaneShuffle`. So that we can reduce the changes?
Assumptions like this is pretty much why i'm having to do this change in the first place...
Could you please specify, *what* specific changes would be simplified by that?
I'm not really seeing it.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103274/new/

https://reviews.llvm.org/D103274