[PATCH] D69891: [VP,Integer,#1] Vector-predicated integer intrinsics

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added a comment.

+1 from me! Thanks for doing this!



================
Comment at: llvm/docs/LangRef.rst:14668
+The explicit vector length parameter always has the type `i32`.
+The explicit vector length is only effective if the MSB of its value is zero.
+Results are only computed for enabled lanes.
----------------
Nit: instead of talking about the MSB being zero, is it simpler to say that it is effective if is a positive number (if that's what you mean)?


================
Comment at: llvm/docs/LangRef.rst:14738
+
+The first two operands and the result have the same vector of integer type. The third operand is the vector mask and has the same number of elements as the result vector type. The fourth operand is the explicit vector length of the operation.
+
----------------
Nit: please ignore if you disagree, but perhaps but perhaps shorter/simpler is:

   The first two operands and the result have the same vector of integer type.

->

   The operands and the result are integer vector types.

(this applies to all/most descriptions)


================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:227
+Optional<int32_t> VPIntrinsic::getStaticVectorLength() const {
+  auto GetStaticVectorLengthOfType = [](const Type *T) -> Optional<int32_t> {
+    auto VT = dyn_cast<VectorType>(T);
----------------
Nit: don't think this lamba adds anything, it's just called once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69891