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

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoZfa.td:58
+let Predicates = [HasStdExtZfa] in {
+def FMINM_S: FPALU_rr<0b0010100, 0b010, "fminm.s", FPR32, /*Commutable*/ 1>;
+def FMAXM_S: FPALU_rr<0b0010100, 0b011, "fmaxm.s", FPR32, /*Commutable*/ 1>;
----------------
craig.topper wrote:
> reames wrote:
> > This is a purely optional comment.
> > 
> > You have the instructions grouped by data type.  It would make the definitions easier to follow if you instead grouped by instruction name.  This would involve slightly more predicate scopes, but would more directly match the wording in the specification.
> I've been wondering if it makes sense to have this file at all or should we fold the instructions into the Zfh, F, and D files?
Either would be reasonable.  Happy to defer to @craig.topper 's judgement here.


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

https://reviews.llvm.org/D141984