xen0n added a comment.

> MOVGR2FCSR modifies the value of the software writable field
> corresponding to the FCSR (floating-point control and status
> register) fcsr according to the value of the lower 32 bits of
> the GR (general purpose register) rj.

The description of `movgr2fcsr` is incorrect, it implies `GPR[rj]`is read, but 
in fact it's only the //`ui5` immediate in `rj` slot//, i.e. `FCSR[rj]`. I 
didn't look very closely but the test case changes seem good.

(It's probably more appropriate to name the instructions `fcsrrd` and `fcsrwr` 
(as done in loongarch-opcodes 
<https://github.com/loongson-community/loongarch-opcodes/blob/develop/la-fp.txt>)
 but it may be hard to officially revise the ISA manual according to some of 
Loongson insiders. Maybe we can use the corrected names in the internals and 
expose the official name for now? This is just a suggestion for your teammates 
to consider though.)



================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:53
+    SDTCisVT<1, GRLenVT>]>;
+def SDT_LoongArchFCSR2GR : SDTypeProfile<1, 1, [SDTCisVT<0, GRLenVT>,
+    SDTCisVT<1, i32>]>;
----------------
Be consistent with the surrounding code and spell out the instruction mnemonic 
fully? Like `SDT_LoongArchMovfcsr2gr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140685

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to