[PATCH] D94708: [IR] Introduce llvm.experimental.vector.splice intrinsic

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Just a few nits, will have a closer look at the patch later.



================
Comment at: llvm/docs/LangRef.rst:16278-16284
+The '``llvm.experimental.vector.splice.*``' intrinsics construct a vector by
+concatenating elements from the first input vector with elements of the second
+input vector, returning a vector of the same type as the input vectors. A
+signed immediate specifies the elements to select from the first vector, where
+the sign of the immediate distinguishes two variants. A positive immediate
+specifies an index into the first vector and a negative immediate specifies the
+number of trailing elements to extract from the first vector.
----------------
nit: 
The signed immediate, modulo the number of elements in the vector, is the index into the first vector from which to extract the result value.
This means conceptually that for a positive immediate, a vector is extracted from concat(%vec1, %vec2) starting at index `imm`, whereas for a negative immediate, it extracts `imm` trailing elements from the first vector, and the remaining elements from %vec2.


================
Comment at: llvm/docs/LangRef.rst:16287
+These intrinsics work for both fixed and scalable vectors. While this intrinsic
+is marked as experimental the recommended way to express this operation for
+fixed-width vectors is still to use a shufflevector, as that may allow for more
----------------
nit: insert comma.


================
Comment at: llvm/docs/LangRef.rst:16305
+elements of the vector. The ``imm`` is a signed integer constant in the range
+``-VL <= imm < VL``, values outside of this range are clamped.
+
----------------
It's better to say that if the immediate value is outside this range, the result is a poison value. That leaves it up to the implementation how to handle it.


================
Comment at: llvm/docs/LangRef.rst:16305
+elements of the vector. The ``imm`` is a signed integer constant in the range
+``-VL <= imm < VL``, values outside of this range are clamped.
+
----------------
sdesmalen wrote:
> It's better to say that if the immediate value is outside this range, the result is a poison value. That leaves it up to the implementation how to handle it.
where VL is the runtime vector length of the source/result vector.


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

https://reviews.llvm.org/D94708