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

fcvt.s.d gives wrong answer #24

Closed
b1f6c1c4 opened this issue Apr 7, 2020 · 15 comments
Closed

fcvt.s.d gives wrong answer #24

b1f6c1c4 opened this issue Apr 7, 2020 · 15 comments
Labels
tooling Toolflow, synthesis and simulation

Comments

@b1f6c1c4
Copy link

b1f6c1c4 commented Apr 7, 2020

I tried to run fcvt.s.d to convert a small double (2.386004908190509275e-40 which is 0x37b4c8f800000000) into single, but it gives inf (0x7f800000) instead of 2.38600491e-40 (0x0002991f).

@stmach
Copy link
Collaborator

stmach commented Apr 10, 2020

Hello,

Have you checked what is effectively applied to the FPU?
Setting up the operation with

op_i       = fpnew_pkg::F2F
rnd_mode_i = fpnew_pkg::RNE
src_fmt_i  = fpnew_pkg::FP64
dst_fmt_i  = fpnew_pkg::FP32

and applying your given input yields 0xffffffff0002991f as a result, which is what you'd expect.
The issue is likely in the integration of the FPU into your system (either configuration or the application of control signals) then. What system are you using the FPU with?

Cheers!

@b1f6c1c4
Copy link
Author

I'm trying with Ariane. How do we debug into it?

@stmach stmach transferred this issue from openhwgroup/cvfpu Apr 10, 2020
@stmach
Copy link
Collaborator

stmach commented Apr 10, 2020

Can you please provide a minimal working code snippet that you execute on Ariane which triggers this issue, so that we may reproduce it?

@b1f6c1c4
Copy link
Author

I saw this error when I was trying to run riscv-tests (https://github.com/riscv/riscv-tests/blob/master/isa/rv64ud/recoding.S) with OpenPiton.

@zarubaf
Copy link
Contributor

zarubaf commented Apr 14, 2020

Can you check the signals of the FPU, please?
@b1f6c1c4 and @stmach can we check that you tested the stimuli on the same fpnew source?

@stmach
Copy link
Collaborator

stmach commented Apr 14, 2020

I've used the latest tagged release v0.6.1 of the FPU but there haven't been changes to the conversion units in a long time, so it should work with older versions as well.

@zarubaf
Copy link
Contributor

zarubaf commented Apr 14, 2020

We have rv64ud-p-recoding in the CI, so I don't think it fails for the repo here anyway.

@b1f6c1c4
Copy link
Author

Here's what I got. BTW, I'm using fpnew v0.6.1 (4a241d4).

shot-2020-04-14_19-43-49

shot-2020-04-14_19-42-57

@zarubaf
Copy link
Contributor

zarubaf commented Apr 15, 2020

@stmach Can this be a VCS/Modelsim difference? When compiling the FPU I am getting two warnings with the FPU in VCS so that might be related (but I don't think so):

Warning-[ENUMASSIGN] Illegal assignment to enum variable
/scratch/zarubaf/snitch/src/fpnew/src/fpnew_opgroup_block.sv, 126
fpnew_opgroup_block, "fpnew_pkg::get_first_enabled_multi(10'b1010101010, FpFmtMask)"
  Only expressions of the enum type can be assigned to an enum variable.
  The type bit [9:0] is incompatible with the enum 'unit_type_t'
  Expression: 10'b1010101010
  Use the static cast operator to convert the expression to enum type.


Warning-[ENUMASSIGN] Illegal assignment to enum variable
/scratch/zarubaf/snitch/src/fpnew/src/fpnew_opgroup_block.sv, 126
fpnew_opgroup_block, "fpnew_pkg::get_first_enabled_multi(10'b1010101010, FpFmtMask)"
  Only expressions of the enum type can be assigned to an enum variable.
  The type bit [9:0] is incompatible with the enum 'unit_type_t'
  Expression: 10'b1010101010
  Use the static cast operator to convert the expression to enum type.

Bender supports generating VCS scripts now and its relatively easy to bring up the simulation in VCS as well.

@zarubaf
Copy link
Contributor

zarubaf commented Apr 15, 2020

@b1f6c1c4 What's your VCS version? Just to be sure we are not hunting bugs in the FPU although they are in the CPU (wrapper for example). Can you provide us with the same screenshot but signals from the fpnew_top instance (https://github.com/pulp-platform/ariane/blob/9cebe8a238aa8e1450c085f051fefc1e6d55cc41/src/fpu_wrap.sv#L513-L538)?

@stmach
Copy link
Collaborator

stmach commented Apr 15, 2020

I tried to run the operation in VCS and indeed i receive Inf (0x7f800000).
@zarubaf I added a static cast to int to solve your warning so it shouldn't be the reason for this mismatch. Feel free to move this issue back to fpnew.
I'll try to find out what triggers VCS to produce the wrong result here... It might take a while though as VCS is not our main simulation target.

@b1f6c1c4
Copy link
Author

FYI

vcs script version : O-2018.09
machine name = della2.princeton.edu
machine type = linux64
machine os = Linux 3.10.0-1062.12.1.el7.x86_64
The FLEXlm host ID of this machine is "80000208fe80 00219ba4ae77 00219ba4ae79 00219ba4ae7b 00219ba4ae7d"
Compiler version = VCS-MX O-2018.09-SP2_Full64
VCS Build Date = Feb 28 2019 22:34:30

@zarubaf zarubaf transferred this issue from openhwgroup/cva6 Apr 15, 2020
@stmach stmach added the tooling Toolflow, synthesis and simulation label Apr 15, 2020
@stmach
Copy link
Collaborator

stmach commented Apr 15, 2020

A post-synthesis netlist obtained from Synopsys DC yields the correct result (simulated with Modelsim since I don't have VCS set up), so the issue seems contained to VCS for now.

@b1f6c1c4
Copy link
Author

b1f6c1c4 commented May 5, 2020

Any update on this?

@b1f6c1c4
Copy link
Author

@stmach @zarubaf Well I'm wondering how can I help for this issue. Will it be helpful if we could agree on a certain version (commit SHA-1) of Ariane and we run VCS and ModelSim separately, and then compare their results? Due to some limitation (broken ModelSim license) I cannot do this on my end. So if you think it will help, you can give me a SHA-1 and I'll do VCS on my end. Otherwise you can debug yourself.

b1f6c1c4 added a commit to b1f6c1c4/fpnew that referenced this issue May 31, 2020
There was a signedness bug in fpnew_cast_multi that causes VCS to behave
incorrectly when converting a small double into float. This fixes openhwgroup#24.
@stmach stmach closed this as completed in 79d8359 Jun 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tooling Toolflow, synthesis and simulation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants