[PATCH] D144670: [AArch64][GlobalISel] Legalize G_SHUFFLE_VECTOR with smaller dest size

[Amara Emerson via Phabricator via llvm-commits]

aemerson accepted this revision.
aemerson added inline comments.
This revision is now accepted and ready to land.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4993
 
-  // TODO: Normalize the shuffle vector since mask and vector length don't
-  // match.
-  if (MaskNumElts <= SrcNumElts) {
+  if (MaskNumElts == SrcNumElts) {
     return LegalizerHelper::LegalizeResult::UnableToLegalize;
----------------
dzhidzhoev wrote:
> paquette wrote:
> > Add a TODO?
> For the purpose of this function (making lengths of G_SHUFFLE_VECTOR instruction arguments equal), I think it should return Legalized if lengths are equal.
no need for braces for the `if` here


================
Comment at: llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp:4999
+    // undef values.
+    SmallVector<int, 8> NewMask(Mask);
+    for (unsigned I = MaskNumElts; I < SrcNumElts; ++I) {
----------------
dzhidzhoev wrote:
> paquette wrote:
> > Why 8 vs default value for `SmallVector`?
> This value is used in SelectionDAGBuilder::visitShuffleVector for the mask, that's why I used it here. Should I use the default value?
I actually think it's worth going up to 16 here, because it's quite common to have mask lengths of that length if you have 8 x 16b elements in a 128b vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144670