[PATCH] D113439: [RISCV] Add IR intrinsics for reading/write vxrm.
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 18 09:28:53 PST 2021
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
More information about the llvm-commits
mailing list