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

Mon May 31 00:44:28 PDT 2021

lebedev.ri added a comment.

Thank you for taking a look!

================
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:
> lebedev.ri wrote:
> > 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.
> I meant if we define it like
> ```
> def FeatureFastVariableShuffle
>     : SubtargetFeature<"fast-variable-shuffle",
>                        "HasFastVariableShuffle",
>                        "true", "Shuffles with variable masks are fast",
>                        [FeatureFastVariablePerLaneShuffle]>;
> ```
> then we can keep most use of `FeatureFastVariableShuffle` unchanged.
> Anyway, it is just nitpicking.
I think the current status is more consistent and perhaps more future-prof/testable,
but i don't think i care enough to argue. I just want to make forward progress with the least latency :)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103274