[PATCH] D123988: [LoongArch] Add basic floating-point instructions definition

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Looks reasonable to me. Some nits on file naming and definition placement. Thanks!



================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrFormatsF.td:1
+//==- LoongArchInstrFormatsF.td - LoongArch FP Instr Formats -*- tablegen -*-=//
+//
----------------
nit: `...InstrFormatF` is a bit cryptic. Perhaps `...FloatInstrFormat`?


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfoD.td:1
+//=-- LoongArchInstrInfoD.td - Double-Precision Float instr -*- tablegen -*-==//
+//
----------------
Same nit on file naming.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfoF.td:22
+
+class FP_ALU_3R<bits<17> op, string opstr, RegisterClass rc>
+    : FPFmt3R<op, (outs rc:$fd), (ins rc:$fj, rc:$fk), opstr, "$fd, $fj, $fk">;
----------------
These definitions are also used in the double file, but not included there and probably only works because you include this one first on the main TD file.

I'd move all common logic to `...InstrFormat` and include that on both `...InstrInfo` files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123988