[PATCH] D93013: [RISCV] Define vadd intrinsics and lower to V instructions.

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:17
 
+def NoX0 : SDNodeXForm<undef,
+[{
----------------
Documenting this and why it's necessary (X0 != ADD X0, 0 for vsetvli) would be good


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:76
+
+class ToFPR32<ValueType type, DAGOperand operand, string name> {
+  dag ret = !cond(!eq(!cast<string>(operand), !cast<string>(FPR64)):
----------------
Documenting this helper class would be helpful


================
Comment at: llvm/test/CodeGen/RISCV/rvv/vadd.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -mtriple=riscv64 -mattr=+m,+f,+d,+a,+c,+experimental-v \
+; RUN:   -verify-machineinstrs --riscv-no-aliases < %s \
----------------
Having `+m,+f,+d,+a,+c` all seem unnecessary here


================
Comment at: llvm/test/CodeGen/RISCV/rvv/vadd.ll:6
+
+declare <vscale x 1 x i8> @llvm.riscv.vadd.nxv1i8.nxv1i8(
+  <vscale x 1 x i8>,
----------------
The formatting of this IR function declaration looks a little weird to me - is this the usual style?


================
Comment at: llvm/test/CodeGen/RISCV/rvv/vadd.ll:18
+entry:
+  %a = call <vscale x 1 x i8> @llvm.riscv.vadd.nxv1i8.nxv1i8(
+    <vscale x 1 x i8> undef,
----------------
It would be good to have a test without `undef` inputs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93013