CSR instructions and Spike bugs

[Andy Wright]

Hey everyone,

I've been going over some details of the spec, and I found a few corners in the ISA that could use some more clarification/fixes resulting in some possible bug fixes in spike

1) Page 8 of v1.7 of the ISA spec suggests that CSRRS, CSRRC, CSRRWI, CSRRSI, and CSRRCI shouldn't write to the CSR if rs1/zimm == 0. CSRRWI shouldn't be included on this list since it is used to implement the CSRWI pseudo-instruction and we already have other instructions that can be used to perform a CSR read without a write. I think the ISA specification should clarify that only CSRRS, CSRRC, CSRRSI, and CSRRCI don't perform writes when rs/zimm == 0.

Right now spike doesn't behave correctly for CSR instructions that shouldn't write. I fixed this behavior in a fork of spike with this commit:

https://github.com/csail-csg/riscv-isa-sim/commit/68da26c33ff74b447801c8adf6dc12c8040ca99e

2) FS is used to track whether the FPU is enabled, and if the FPU state has been dirtied. It should probably be clarified that FPU state includes f0-f31, fflags, frm, and fcsr, but it also needs some more clarification of when you _can_ set FS to dirty and when you _must_ set FS to dirty.

When anything in the FPU state changes, FS should be set to dirty. But what if something in the FPU state was written with the same value? For example, fflags was read and cleared (CSRRW) but it was already zero. Another interesting corner with this is FCVT instructions that convert floating point values to integers. They don't write to the FPU register file but they can still raise fflags (as I understand it). Do FCVT instructions always set FS to dirty? only when FCVT sees a floating point exception? only when FCVT raises a new floating point exception not already in fflags?

I think implementations should be required to set FS to dirty when the value of the FPU state changes, but the implementations should be free to set FS to dirty in cases where the value didn't change. If it was legal for an implementation to set FS to dirty at will (assuming FS was not set to disabled), then you could have a simplified FS implementation that any non-zero write to FS sets FS as dirty so no later updates to FS are required.

Right now spike sets FS to dirty when an FPU CSR is written through a CSRRW-like instruction, and when an instruction writes to an FPU register. It doesn't set FS to dirty when fflags changes due to FCVT converting a float to an integer. I fixed this by setting FS to dirty whenever the value of fflags changes (the most aggressive solution -- if FCVT raises a flag that is already set in fflags, then FS will not be set to dirty). The commit for this fix is here:

https://github.com/csail-csg/riscv-isa-sim/commit/f56f2b7b243415e034c9e583bdbdb9eb2be9709f

If these both of the spike fixes make sense, then I can send a pull request, but if both of these have been nullified by a drastically different v1.9 supervisor spec, then I would really like to see a draft.

-Andy

[Andrew Waterman]

Both fixes look correct, but the FCVT fix is inconsistent with how
Spike handles other FS updates. In the other cases, it sets FS to
dirty on any write, even if the value was the same. Both are legal
implementations, as you point out, but I'd probably set FS more
sloppily for consistency.

If you submit a PR, I'll merge it. Thanks.

> --
> You received this message because you are subscribed to the Google Groups
> "RISC-V ISA Dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to isa-dev+u...@groups.riscv.org.
> To post to this group, send email to isa...@groups.riscv.org.
> Visit this group at
> https://groups.google.com/a/groups.riscv.org/group/isa-dev/.
> To view this discussion on the web visit
> https://groups.google.com/a/groups.riscv.org/d/msgid/isa-dev/CAC1Xjkd2yeRhfxCa4sJLakCWxFv%3DpnG1st7QQQT6wzWA6PD%3DZQ%40mail.gmail.com.

[Andy Wright]

Thanks. Just sent the PR. I changed the behavior of the FCVT fix to only set FS to dirty if softfloat_exceptionFlags is non-zero. I think it fits in better with spike and hardware implementations.

Andy

To view this discussion on the web visit https://groups.google.com/a/groups.riscv.org/d/msgid/isa-dev/CA%2B%2B6G0Cp1xMu0yicttOnPPiE75PcpncdbHZ-5uqZbwndygZguw%40mail.gmail.com.