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

[Simon Moll via Phabricator via llvm-commits]

simoll planned changes to this revision.
simoll marked 4 inline comments as done.
simoll added a comment.

In D69891#1745257 <https://reviews.llvm.org/D69891#1745257>, @SjoerdMeijer wrote:

> +1 from me! Thanks for doing this!


Thanks for reviewing! I'll upload a final update for the LangRef.



================
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.
----------------
SjoerdMeijer wrote:
> 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)?
Good point. I'll rewrite that part referring to the vlen as a signed integer.


================
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.
+
----------------
SjoerdMeijer wrote:
> 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)
I'd like to shorten that. However, there are always mask and vlen operands so we cannot say that all operands have an integer vector type.


================
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);
----------------
SjoerdMeijer wrote:
> Nit: don't think this lamba adds anything, it's just called once.
It'll be called twice in the final version (see [reference patch](https://reviews.llvm.org/D57504)).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69891