[PATCH] D105351: [VP] Declaration and docs for vp.select intrinsic

[Simon Moll via Phabricator via llvm-commits]

simoll added inline comments.


================
Comment at: llvm/docs/LangRef.rst:17753
+
+The first operand is a vector of ``i1`` and indicates the mask.  The second
+operand is the value that is selected when the condition is true.  The third
----------------
frasercrmck wrote:
> Nit: the `select` instruction is defined as having a "condition" whereas this is a "mask". There's a bit of mixing of "mask" and "condition" in the follow paragraphs too. Should we try to stick to "condition"?
+1 for consistency. The difference to regular `select` is that the condition/mask has to be a vector. How about "condition vector"?


================
Comment at: llvm/include/llvm/IR/Intrinsics.td:1510
 }
+// Shuffles.
+def int_vp_select : DefaultAttrsIntrinsic<[ llvm_anyvector_ty ],
----------------
frasercrmck wrote:
> Nit: is this really a shuffle? I guess in a way it is, but it feels like "shuffles" and "selects" are already two distinct terms in the non-VP world.
I think it's nice to group all shuffle intrinsics together (intrinsics that rearrange lanes). Actually, `vp.splice` and the (upcoming) `vp.merge/thePivotThing` should be in the shuffle category as well.
It's pretty inconsequential anyway as this understanding of "shuffle" only exists in these source code comments and does not appear in the docs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105351