NaN boxing issue for floating point

[UrvishkumarPatel]

While D extension is enabled, according to the Unprivileged ISA V20191213, if for some instruction source register is a single-precision floating value, then a check must be made to see if the upper 32 bits of float register are all 1. This check is not there in the whisper.

These are the exact words from '12.2 NaN Boxing of Narrower Values' in spec:

Apart from transfer operations described in the previous paragraph, all other floating-point operations on narrower n-bit operations, n < FLEN, check if the input operands are correctly NaN-boxed, i.e., all upper FLEN−n bits are 1. If so, the n least-significant bits of the input are used as the input value, otherwise the input value is treated as an n-bit canonical NaN.

[jrahmeh]

Hi Urvish,

The check you mention is supposed to be there. If it does not always get applied then that is a bug. Please share the test case exhibiting the bug and I will fix it.

Joe

Here's the code that does the check (file FpRegs.hpp line 356): template<> inline float FpRegs::readSingle(unsigned i) const { FpUnion u; u.dp = regs_.at(i); // Check for proper nan-boxing. If not properly boxed, replace with NaN. if (~u.sp.pad != 0) return std::numeric_limits::quiet_NaN(); return u.sp.sp; }

On Wed, Feb 24, 2021 at 11:20 PM Urvish Patel notifications@github.com wrote:

While D extension is enabled, according to the Unprivileged ISA V20191213, if for some instruction source register is a single-precision floating value, then a check must be made to see if the upper 32 bits of float register are all 1. This check is not there in the whisper.

These are the exact words from '12.2 NaN Boxing of Narrower Values' in spec:

Apart from transfer operations described in the previous paragraph, all other floating-point operations on narrower n-bit operations, n < FLEN, check if the input operands are correctly NaN-boxed, i.e., all upper FLEN−n bits are 1. If so, the n least-significant bits of the input are used as the input value, otherwise the input value is treated as an n-bit canonical NaN.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/chipsalliance/SweRV-ISS/issues/7, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASHQXYHF5HCRGW637SHLDTTAXMYDANCNFSM4YF2LBLQ .

[UrvishkumarPatel]

Hi Joe, Thanks for the response. Actually, I was using an older version, which was not having this fix (Also, does the latest commit take into account that NaN-boxing is not checked for instructions like fsw and fcvt.x.w. These instructions must ignore the upper FLEN-n bits).

But when I compiled the latest commit and ran the following command, it showed an unexpected behaviour:

$ ./build-Linux/whisper --isa imafdc --interactive
whisper> poke m 0 0x53
whisper> s
#1 0 00000000 00000053 c 0300       00001800  fadd.s   f0, f0, f0, rne  +
#1 0 00000000 00000053 c 0341       00000000  fadd.s   f0, f0, f0, rne  +
#1 0 00000000 00000053 c 0342       00000002  fadd.s   f0, f0, f0, rne  +
#1 0 00000000 00000053 c 0343       00000053  fadd.s   f0, f0, f0, rne
whisper>

[jrahmeh]

Hi Urvish,

We implemented the FS bits of mstatus. The default value for those bits is 00 (off). To enable floating point you need to set them to 01: poke c mstatus 0x3800 poke m 0 0x53 s

Alternatively you change the reset value of mstatus by creating a config file, say whisper.json and putting the following in it: { "csr" : { "mstatus" : { "reset" : "0x3800", "exists" : "true", "mask" : "0x6088" } } } Then you can run like this: whisper --configfile whisper.json --isa imacfd -i poke m 0 0x53 s

Or you can enable newlib emulation (which enables extensions a,f, and d and turns on floating point in mstatus): whisper --newlib -i poke m 0 0x53 s

Joe

On Fri, Feb 26, 2021 at 2:33 AM Urvish Patel notifications@github.com wrote:

Hi Joe, Thanks for the response. Actually, I was using an older version, which was not having this fix (Also, does the latest commit take into account that NaN-boxing is not checked for instructions like fsw and fcvt.x.w. These instructions must ignore the upper FLEN-n bits).

But when I compiled the latest commit and ran this, it showed an unexpected behaviour:

$ ./build-Linux/whisper --isa imafdc --interactive whisper> poke m 0 0x53 whisper> s

1 0 00000000 00000053 c 0300 00001800 fadd.s f0, f0, f0, rne +

1 0 00000000 00000053 c 0341 00000000 fadd.s f0, f0, f0, rne +

1 0 00000000 00000053 c 0342 00000002 fadd.s f0, f0, f0, rne +

1 0 00000000 00000053 c 0343 00000053 fadd.s f0, f0, f0, rne

whisper>

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/chipsalliance/SweRV-ISS/issues/7#issuecomment-786494771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASHQXYKYF7GNCZDQQZMO5DTA5MGDANCNFSM4YF2LBLQ .

[UrvishkumarPatel]

Hi Joe, Are 's' and 'u' (supervisor and user mode) supported by newlib emulation?

Urvish

[jrahmeh]

They should be

On Mon, Jul 12, 2021, 8:08 AM Urvish Patel @.***> wrote:

Hi Joe, Are 's' and 'u' (supervisor and user mode) supported by newlib emulation?

Urvish

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/chipsalliance/SweRV-ISS/issues/7#issuecomment-878264346, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASHQXYNBIAAQQ5CPCWJSOLTXLSLZANCNFSM4YF2LBLQ .