[PATCH] D149916: [VP][SelectionDAG][RISCV] Add get_vector_length intrinsics and generic SelectionDAG support.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/docs/LangRef.rst:18064
+
+      declare i32 @llvm.experimental.get.vector.length.i32(i32 %cnt,, i32 %element_width, i32 %vf)
+      declare i32 @llvm.experimental.get.vector.length.i64(i64 %cnt, i32 %element_width, i32 %vf)
----------------
Double comma here


================
Comment at: llvm/docs/LangRef.rst:18065
+      declare i32 @llvm.experimental.get.vector.length.i32(i32 %cnt,, i32 %element_width, i32 %vf)
+      declare i32 @llvm.experimental.get.vector.length.i64(i64 %cnt, i32 %element_width, i32 %vf)
+
----------------
Should we explicitly denote this intrinsic with `immarg` parameters in the LangRef? I only see the one doing it but it sounds kind helpful


================
Comment at: llvm/docs/LangRef.rst:18079
+to be processed. The second argument is i32 immediate for the data type and is
+in the range 8 <= imm <= supported element width. This serves as a hint to the
+target about the element types. The third parameter is an i32 immediate for the
----------------
Maybe also saying that the element width is in bits would clarify things a little better.


================
Comment at: llvm/docs/LangRef.rst:18079
+to be processed. The second argument is i32 immediate for the data type and is
+in the range 8 <= imm <= supported element width. This serves as a hint to the
+target about the element types. The third parameter is an i32 immediate for the
----------------
frasercrmck wrote:
> Maybe also saying that the element width is in bits would clarify things a little better.
If it's just a hint, do we need to restrict it to being at least 8? It might feel a little artificial.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149916