[PATCH] D100865: [COST] Improve shuffle kind detection if shuffle mask is provided.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:741
 
+  TTI::ShuffleKind deduceShuffleKindFromMask(TTI::ShuffleKind Kind,
+                                             ArrayRef<int> Mask) const {
----------------
nit: maybe `improveShuffleKindFromMask` is more descriptive?


================
Comment at: llvm/include/llvm/CodeGen/BasicTTIImpl.h:743-757
+    if (Mask.empty())
+      return Kind;
+    if (Kind == TTI::SK_PermuteSingleSrc) {
+      if (ShuffleVectorInst::isReverseMask(Mask))
+        return TargetTransformInfo::SK_Reverse;
+      if (ShuffleVectorInst::isZeroEltSplatMask(Mask))
+        return TargetTransformInfo::SK_Broadcast;
----------------
nit: This is probably just personal preference, but can this be implemented with a switch? i.e.

  if (Mask.empty())
    return Kind;

  switch (Kind) {
  default:
    break;
  case Kind == TTI::SK_PermuteSingleSrc:
    if (ShuffleVectorInst::isReverseMask(Mask))
      return TargetTransformInfo::SK_Reverse;
    if (ShuffleVectorInst::isZeroEltSplatMask(Mask))
      return TargetTransformInfo::SK_Broadcast;
    break;
  case TTI::SK_PermuteTwoSrc:
    if (ShuffleVectorInst::isSelectMask(Mask))
      return TargetTransformInfo::SK_Select;
    if (ShuffleVectorInst::isTransposeMask(Mask))
      return TargetTransformInfo::SK_Transpose;
    break;
  }
  return Kind;


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:4269
     Value *V = TE->Scalars[I];
+    if (isa<UndefValue>(V))
+      continue;
----------------
Is this change supposed to be in this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100865