[PATCH] D105351: [VP] Declaration and docs for vp.select intrinsic
Simon Moll via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 19 23:41:27 PDT 2021
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
More information about the llvm-commits
mailing list