[PATCH] D141984: [RISCV][MC] Add support for experimental zfa extension(FLI instruction not included)

Craig Topper via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 21:19:00 PST 2023


craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZfa.td:10
+// This file describes the RISC-V instructions from the standard 'Zfa'
+// additional floating-point extension, version 1.0.
+// This version is still experimental as the 'Zfa' extension hasn't been
----------------
The spec says 0.1 I think.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZfa.td:31
+let Predicates = [HasStdExtZfa] in {
+defm FMINM_S: FPALU_rr_m<0b0010100, 0b010, "fminm.s", FINX, /*Commutable*/ 1>;
+defm FMAXM_S: FPALU_rr_m<0b0010100, 0b011, "fmaxm.s", FINX, /*Commutable*/ 1>;
----------------
I don't think you want to pass FINX here. That will create additional versions that use GPR has registers like Zfinx.

I think you should use `FPALU_rr` instead.

Similar issues exist for other instructions.


================
Comment at: llvm/test/MC/RISCV/rv32zfa-only-valid.s:1
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+experimental-zfa,+d,+zfh -riscv-no-aliases -show-encoding \
+# RUN:     | FileCheck -check-prefixes=CHECK-ASM,CHECK-ASM-AND-OBJ %s
----------------
Please add -invalid tests as well to make sure these instructions are not accepted without Zfa.


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

https://reviews.llvm.org/D141984



More information about the llvm-commits mailing list