[PATCH] D115329: [LoopVectorize] Pass a vector type to isLegalMaskedGather/Scatter

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

@lebedev.ri/@fhahn given that there exists a pass that scalarizes the masked load/store/gather/scatter intrinsics, is there a reason why we wouldn't always want to choose the intrinsic representation over scalarizing it in the LV? It seems a bit odd to want to trick the LV to think that a masked gather is legal, but then still scalarize it in the end. This means it is not using the right cost-model, unless it reimplements the scalarization costs, like is done by `X86TargetTransformInfo::getGSScalarCost`.



================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.h:194
+    // For MVE, we have a custom lowering pass that will already have custom
+    // legalised any gathers that we can to MVE intrinsics, and want to expand
+    // all the rest. The pass runs before the masked intrinsic lowering pass, so
----------------
nit: s/can to/can lower to/


================
Comment at: llvm/lib/Target/ARM/ARMTargetTransformInfo.h:195-196
+    // legalised any gathers that we can to MVE intrinsics, and want to expand
+    // all the rest. The pass runs before the masked intrinsic lowering pass, so
+    // if we are here, we know we want to expand.
+    return true;
----------------
nit: s/, so if we are here, we know we want to expand//


================
Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:5132
+  // case.
+  unsigned NumElts = cast<FixedVectorType>(VTy)->getNumElements();
+  return NumElts == 1 ||
----------------
This is casting to `FixedVectorType`, so I think we should we make the interface take `VectorType` instead of `Type`.


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

https://reviews.llvm.org/D115329