[PATCH] D99750: [LV, VP] RFC: VP intrinsics support for the Loop Vectorizer (Proof-of-Concept)

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/include/llvm/IR/IRBuilder.h:2545
+  /// Return an all false boolean vector of size and scalability \p NumElts.
+  Value *CreateFalseVector(ElementCount NumElts) {
+    VectorType *VTy = VectorType::get(Type::getInt1Ty(Context), NumElts);
----------------
This method doesn't seem used by this patch.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3020
+                 "Mask argument is required for VP intrinsics.");
+          VectorType *StoredValTy = cast<VectorType>(StoredVal->getType());
+          Value *BlockInMaskPart = BlockInMaskParts[Part];
----------------
Does this need to be a `VectorType`? Can't you pass `StoredVal->getType()` straight through to `CreateIntrinsic`?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4998
+
+  // Just widen unops and binops.
+  setDebugLocFromInst(Builder, &I);
----------------
Is this comment out of place?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8651
 
-  if (!LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range))
+  return (LoopVectorizationPlanner::getDecisionAndClampRange(willWiden, Range));
+}
----------------
Unnecessary parens around this statement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99750