[PATCH] D121376: [RISCV][RVV] Introduce roundmode operand to PseudoVAADD instruction

[Fraser Cormack via Phabricator via llvm-commits]

frasercrmck added inline comments.


================
Comment at: llvm/include/llvm/IR/IntrinsicsRISCV.td:639
   }
+  // For Saturating binary operations with mask.
+  // The destination vector type is the same as first source vector.
----------------
Is it worth saying `with mask and rounding-mode operand` to help people differentiate between the intrinsics?

Also this one is defined after the no-roundmode version, but `RISCVSaturatingBinaryRMAAXNoMask` is defined //before// its counterpart. I find that quite confusing.


================
Comment at: llvm/lib/Target/RISCV/CMakeLists.txt:29
   RISCVInsertVSETVLI.cpp
+  RISCVVXRMRegister.cpp
   RISCVInstrInfo.cpp
----------------
Nit: in alphabetical order, please


================
Comment at: llvm/lib/Target/RISCV/RISCVVXRMRegister.cpp:21
+//
+// This can be enhanced later if the ABI specify it is volatile across calls.
+//
----------------
specify -> specifies


================
Comment at: llvm/lib/Target/RISCV/RISCVVXRMRegister.cpp:101
+    auto NewVXRM = hasVXRMInfo(MI);
+    if (NewVXRM) {
+      unsigned NewVXRMImm = NewVXRM.getValue();
----------------
`if (auto NewVXRM = hasVRXMInfo(MI))`


================
Comment at: llvm/lib/Target/RISCV/RISCVVXRMRegister.cpp:151
+    } else {
+      // it is a fallthrou to next block
+      BuildMI(&MBB, MI.getDebugLoc(), TII->get(RISCV::WriteVCSR))
----------------
`fallthrough`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121376