[PATCH] D99083: [RISCV] Introduce floating point control and state registers

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1173
 
+def ReadFRM : ReadSysReg<SysRegFRM, [FRM]>;
+def WriteFRM : WriteSysReg<SysRegFRM, [FRM]>;
----------------
sepavloff wrote:
> sepavloff wrote:
> > craig.topper wrote:
> > > Do we have use cases for all of these?
> > `ReadFRM` is used in the implementation of FLT_ROUNDS_ (D90854).
> > `WriteFRM` and `WrireFRMImm` are used in the implementation of set_rounding (D91242).
> > `SwapFRM` and `SwapFRMImm` are not used in any of pending patches but they are useful to implement target-specific optimization of the code pattern:
> > ```
> >     int old_rm = fegetround();
> >     fesetround(new_rm);
> >     ...
> >     fesetround(old_rm);
> > ```
> > which is likely to be used in the implementation of `#pragma STDC FENV_ROUND`.
> > 
> > Operations with FFLAGS are not used now. They would be needed to implement functions like `fetestexcept` and similar, but now there are no such attempts AFAIK.
> > 
> > Operations would be needed for target specific implementation of intrinsics that access FP control modes (D82525) and FP environment (D71742).
> > Operations would be needed for target specific implementation of intrinsics that access FP control modes (D82525) and FP environment (D71742).
> 
> It is about the operation on `FCSR`.
I don't think we want unused pseudos that might be used someday. We should include them when they're needed.


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:543
+// Special registers
+def FFLAGS : RISCVReg<1, "fflags">;
+def FRM    : RISCVReg<2, "frm">;
----------------
sepavloff wrote:
> craig.topper wrote:
> > Is there any significance the 1, 2, and 3 chosen for the first operand here?
> The first operand of `RISCVReg` is `HWEncoding`, 1, 2 and 3 represent hardware addresses of the respective system registers which are specified in the RISCV specification (https://github.com/riscv/riscv-isa-manual/releases/download/draft-20210402-1271737/riscv-spec.pdf), chapter 25 (RV32/64G Instruction Set Listings), table 25.3 (RISC-V control and status register (CSR) address map).
> 
> On the other hand, these special registers are not used as operands in any of non-pseudo instruction, so these definitions are not used in instruction emitter. It means that particular values are not important, at least now.
I think these should probably just be 0 like the flags registers on other targets. It's only a 5 bit field and the CSR addresses are 12 bits so its purely a coincidence that these CSRs happen to be representable in 5 bits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99083