[PATCH] D113439: [RISCV] Add IR intrinsics for reading/write vxrm.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.td:1338
 
+let hasSideEffects = true in {
+def ReadVXRM : ReadSysReg<SysRegVXRM, [VXRM]>;
----------------
rogfer01 wrote:
> Next to the `FRM` above this makes me curious. Either we don't need this or we forgot it for the `FRM` ones?
> 
> It is a bit surprising we defined `ReadSysReg` / `WriteSysReg` / `WriteSysRegImm` as not having side effects. I wonder what is the reason for that.
> 
> I'm sure I'm missing something here.
FRM is incomplete. None of the FP instructions are annotated to with Uses/Defs of FRM.

I think we found issues with instruction scheduling for VXRM on our internal branch if we didn't have hasSideEffects here. I'll see if I can find the discussion and if there was a test case added. I suspect FRM also needs it, but hasn't been tested as much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113439