[PATCH] D97406: [Vectorizers]Improve emission of logical or/and reductions.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Hi @ABataev, I saw this patch come by and left some comments, hope that's alright!



================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1031
   auto *SrcVecEltTy = cast<VectorType>(Src->getType())->getElementType();
+  if ((RdxKind == RecurKind::And || RdxKind == RecurKind::Or) &&
+      SrcVecEltTy == Builder.getInt1Ty()) {
----------------
Can you add the condition that `&& isa<FixedVectorType>(Src)`? (same request for LoopVectorize.cpp and SLPVectorize.cpp)

We're starting to make the LoopVectorizer vectorize for scalable VFs. This means we're currently fixing up cases like this where assumptions are made that are only valid for fixed-width vectors. For scalable vectors it might be possible to do the `<vscale x N x i1>` reduction as a compare on `<vscale x 1 x iN>`, but at least for SVE I know that we never want that.


================
Comment at: llvm/lib/Transforms/Utils/LoopUtils.cpp:1034-1036
+        Src, Builder.getIntNTy(cast<VectorType>(Src->getType())
+                                   ->getElementCount()
+                                   .getFixedValue()));
----------------
nit: `cast<FixedVectorType>(Src->getType())->getNumElements()`


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:6779-6794
+  if ((RdxKind == RecurKind::Or || RdxKind == RecurKind::And) &&
+      VectorTy->getElementType() ==
+          IntegerType::getInt1Ty(VectorTy->getContext())) {
+    // Or reduction for i1 is represented as:
+    // %val = bitcast <ReduxWidth x i1> to iReduxWidth
+    // %res = cmp ne iReduxWidth %val, 0
+    // And reduction for i1 is represented as:
----------------
Should this be part of BasicTTImpl::getArithmeticReductionCost? That way you don't have to implement it twice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97406