Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sail incorrectly sets flags on fcvtmod.w.d #388

Closed
davidharrishmc opened this issue Jan 13, 2024 · 3 comments · Fixed by #442
Closed

Sail incorrectly sets flags on fcvtmod.w.d #388

davidharrishmc opened this issue Jan 13, 2024 · 3 comments · Fixed by #442

Comments

@davidharrishmc
Copy link

fcvtmod.w.d sets the flags differently in Sail and Spike on one of the test cases in the draft Zfa riscv-arch-test rv32i_m/D_Zfa/src/fcvtmod.w.d_b22-01.S at inst_34. I believe Sail is incorrect.

Performing fcvtmod.w.d on 0xc1ee9b7e5fc9eba4 sets the inexact flag from Sail, but the invalid flag from Spike. This number is an ordinary negative double-precision floating-point number (-4.10806e+09) that is outside the range that can be represented by a 32-bit 2's complement number. According to the IEEE754-2019 spec at page 39, "When a numeric operand would convert to an integer outside the range of the destination format, the invalid operation exception shall be signaled if this situation cannot otherwise be indicated. When the value of the conversion operation’s result differs from its operand value, yet is representable in the destination format, some conversion operations are specified below to signal the inexact exception and others to not signal the inexact exception." According to the riscv-zfa fcvtmod.w.d spec in Section 26.4, "Floating-point exception flags are raised the same as they would be for FCVT.W.D with the same input operand." Therefore, it seems that the Invalid operation flag should be set, as Spike does.

I've attached a test case called my.elf that was generated by riscof when compiling this test. The issue can be reproduced with:

riscv_sim_RV32 --test-signature=sail.sig my.elf > sail.log
spike --isa=rv32imafdc_Zicsr_Zicond_Zfa_Zfh_Zba_Zbb_Zbc_Zbs +signature=spike.sig +signature-granularity=4 my.elf
diff sail.sig spike.sig
126c126
< 00000001
---
> 00000010

For reference, the test is:

inst_34:// fs1 == 1 and fe1 == 0x41e and fm1 == 0xe9b7e5fc9eba4 and  fcsr == 0x0 and rm_val == 7   
/* opcode: fcvtmod.w.d ; op1:f31; dest:x31; op1val:0xc1ee9b7e5fc9eba4; valaddr_reg:x8;
val_offset:10*FLEN/8; rmval:rtz; correctval:??; testreg:x6;
fcsr_val:0*/
TEST_FPID_OP(fcvtmod.w.d, x31, f31, rtz, 0, 0, x8, 10*FLEN/8, x9, x5, x6,FLREG)

which becomes
80000564 <inst_34>:
80000564:	05043f87          	fld	ft11,80(s0)
80000568:	00000313          	li	t1,0
8000056c:	00331073          	fscsr	t1
80000570:	c28f9fd3          	fcvtmod.w.d	t6,ft11,rtz
80000574:	003024f3          	frcsr	s1
80000578:	03f2ac23          	sw	t6,56(t0)
8000057c:	0292ae23          	sw	s1,60(t0)

which has the following Sail trace

[350] [M]: 0x80000564 (0x05043F87) fld ft11, 80(fp)
mem[R,0x80002120] -> 0xC1EE9B7E5FC9EBA4
f31 <- 0xC1EE9B7E5FC9EBA4
mem[X,0x80000568] -> 0x0313
mem[X,0x8000056A] -> 0x0000
[351] [M]: 0x80000568 (0x00000313) addi t1, zero, 0
x6 <- 0x00000000
mem[X,0x8000056C] -> 0x1073
mem[X,0x8000056E] -> 0x0033
[352] [M]: 0x8000056C (0x00331073) csrrw zero, fcsr, t1
CSR fcsr -> 0x00000001
CSR fcsr <- 0x00000000 (input: 0x00000000)
mem[X,0x80000570] -> 0x9FD3
mem[X,0x80000572] -> 0xC28F
[353] [M]: 0x80000570 (0xC28F9FD3) fcvtmod.w.d t6, ft11
x31 <- 0x0B240D02
mem[X,0x80000574] -> 0x24F3
mem[X,0x80000576] -> 0x0030
[354] [M]: 0x80000574 (0x003024F3) csrrs s1, fcsr, zero
CSR fcsr -> 0x00000001
x9 <- 0x00000001
mem[X,0x80000578] -> 0xAC23
mem[X,0x8000057A] -> 0x03F2
[355] [M]: 0x80000578 (0x03F2AC23) sw t6, 56(t0)
mem[0x80003300] <- 0x0B240D02
mem[X,0x8000057C] -> 0xAE23
mem[X,0x8000057E] -> 0x0292
[356] [M]: 0x8000057C (0x0292AE23) sw s1, 60(t0)
mem[0x80003304] <- 0x00000001

my.elf.gz

@davidharrishmc
Copy link
Author

davidharrishmc commented Mar 7, 2024

The problem seems to be in line 748 of
https://github.com/riscv/sail-riscv/blob/master/model/riscv_insts_zfa.sail:

    /* Raise FP exception flags, honoring the precedence of nV > nX */
    let flags : bits(5) = if (true_exp > 31)         then nvFlag()

This line sets invalid flag if the exponent is 32 or more. As I understand the code, it would set invalid for inputs >= 1.0 x 2^32 or <= -1.0 x 2^32.

According to the Zfa specification, "Floating-point exception flags are raised the same as they would be for FCVT.W.D with the same input operand." According to the unprivileged specification Table 11.4, the invalid exception should be set for inputs > 2^31 - 1 or < -2^31.

@davidharrishmc
Copy link
Author

davidharrishmc commented Mar 7, 2024

For comparison, the relevant spike code is:

https://github.com/riscv-software-src/riscv-isa-sim/blob/581e0da68541a20e6ba385acd2b9dcee2358176e/riscv/insns/fcvtmod_w_d.h#L1

  /* Handle overflows */
  if (true_exp > 31 || frac > (sign ? 0x80000000ull : 0x7fffffff)) {
    /* Overflow, for which this operation raises invalid.  */
    invalid = true;
    inexact = false;  /* invalid takes precedence */
  }

@allenjbaum
Copy link
Collaborator

allenjbaum commented Mar 7, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants