[PATCH] D140100: [SLP]Integrate looking through shuffles logic into ShuffleInstructionBuilder.

[Valeriy Dmitriev via Phabricator via llvm-commits]

vdmitrie added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6292
+protected:
+  /// Checks if the mask is an identity mask.
+  static bool isIdentityMask(ArrayRef<int> Mask, const FixedVectorType *VecTy,
----------------
Please add that when isStrict is true it returns false if mask size does not match vector size.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6297
+    int VF = VecTy->getNumElements();
+    return (VF == Limit || (!IsStrict && VF != Limit)) &&
+           all_of(Mask, [Limit](int Idx) { return Idx < Limit; }) &&
----------------
You can drop " && VF != Limit".


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6318
+  /// Looks through shuffles trying to reduce final number of shuffles in the
+  /// code.
+  static bool peekThroughShuffles(Value *&V, SmallVectorImpl<int> &Mask,
----------------
Could you please give more explanatory description here including arguments and return values?
 


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6329
+        break;
+      // Exit if the identity or broadcast mask is found.
+      if (isIdentityMask(Mask, SVTy, /*IsStrict=*/false)) {
----------------
We do not exit. The comment needs an update.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6338
+      }
+      // Exit if the broadcast mask is found.
+      if (SV->isZeroEltSplat()) {
----------------
We do not exit here too.


================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:6341
+        IdentityOp = SV;
+        IdentityMask.assign(Mask);
+      }
----------------
this needs an explanation why IdentityMask is assigned with Mask


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140100