The fmin test is expecting the wrong result for FMAX instruction when one input is sNAN another one is common value

[zhenbohu]

@aswaterman

Hi, Andrew

I noticed you have updated the riscv-tests/isa/rv64uf/fmin.S in the latest version as below:

FMIN(sNaN, x) = x

TEST_FP_OP2_S(20, fmax.s, 0x10, 1.0, sNaNf, 1.0);

Means, you expect the result is x if the input is a sNaN and x. This seems is not correct, because we can see from the the ISA Spec, it is clearlly stating:

"For FMIN and FMAX, if at least one input is a signaling NaN, or if both inputs are quiet NaNs, the result is the canonical NaN. " --------------------- So the expected result should be sNaN, rather than the x....

And I even noticed you actually set the correct expecation in the previous version, but just changed it to this wrong style in latest version, is there any reason why you did it in this way?

I just tried to use spike to run this test, and I found the spike is also printint the wrong result, so does it means the Spike also implemented it in the wrong style, which is also mismatch with the spec?

Thanks Bob

[aswaterman]

Hi Bob,

This was a change to the spec: https://groups.google.com/a/groups.riscv.org/forum/#!searchin/isa-dev/fmin%7Csort:relevance/isa-dev/AKZoOuuZf-I/OvuQck0UAwAJ

[aswaterman]

https://github.com/riscv/riscv-isa-manual/commit/cd20cee7efd9bac7c5aa127ec3b451749d2b3cce

[zhenbohu]

@aswaterman

Hi, Andrew

Thanks for your quick response, since in the user ISA spec v2.2, it was marking the F and D extension as frozen. I thought it will never be changed any more.

So the spec change you pointed above, will be reflected in the next official user ISA SPEC release, v2.3?

Thanks Bob

[aswaterman]

Indeed, a few NaN-related changes have been made, which technically violate the frozenness of the ISA spec. The feeling of the community was that the improvements to the ISA were justified, since few (if any) user-programmable hardened RISC-V FPUs are shipping.