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

Alex Bradbury via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 9 10:54:38 PST 2023


asb accepted this revision.
asb 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>;
----------------
reames wrote:
> 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.
No strong opinion for on keeping this separate or merging into another file. At least for the G extensions, the instruction definitions are in the order they're listed in the RV32/64G Instruction Set Listings table, on the basis this makes it easier to check at a glance if the right encodings are used. Sadly a similar table doesn't seem to exist in the documentation for Zfa.


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

https://reviews.llvm.org/D141984



More information about the llvm-commits mailing list