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

[ShihPo Hung via Phabricator via llvm-commits]

arcbbb added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVVXRMRegister.cpp:108
+        assert(SavedVCSRReg == 0);
+        unsigned NewVCSRImm = NewVXRMImm << 1;
+        SavedVCSRReg = MRI->createVirtualRegister(&RISCV::GPRRegClass);
----------------
khchen wrote:
> sorry, I didn't understand why we need to `<< 1`?
> 
> ```
> enum RoundingMode {
>   RNU = 0,
>   RNE = 1,
>   RDN = 2,
>   ROD = 3,
>   DYN = 4,
> };
> ```
> if `NewVXRMImm` is `RDN` then the `CurVXRMImm` would be `DYN` after `<<1`?
This is doing `VCSR = VXRM << 1`


================
Comment at: llvm/test/CodeGen/RISCV/rvv/vaadd-rm-rv32.ll:3
+; RUN: llc -mtriple=riscv32 -mattr=+v -verify-machineinstrs \
+; RUN:   < %s | FileCheck %s
+declare <vscale x 1 x i8> @llvm.riscv.vaadd.rm.nxv1i8.nxv1i8(
----------------
khchen wrote:
> nit: maybe we could merge `vaadd-rm-rv32.ll`and `vaadd-rm-rv64.ll` into one as we usually did in other tests.
> ex. https://github.com/llvm/llvm-project/blob/main/llvm/test/CodeGen/RISCV/rvv/vaadd.ll#L1-L5
> ```
> ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
> ; RUN: sed 's/iXLen/i32/g' %s | llc -mtriple=riscv32 -mattr=+v \
> ; RUN:   -verify-machineinstrs | FileCheck %s --check-prefixes=CHECK,RV32
> ; RUN: sed 's/iXLen/i64/g' %s | llc -mtriple=riscv64 -mattr=+v \
> ; RUN:   -verify-machineinstrs | FileCheck %s --check-prefixes=CHECK,RV64
> ```
> 
> I think it could be in anorther NFC patch.
Thanks, I'll try merge them in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121376