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

[BUG] NX flag not correctly triggered in specific cases #139

Open
fly-1011 opened this issue Aug 22, 2024 · 7 comments
Open

[BUG] NX flag not correctly triggered in specific cases #139

fly-1011 opened this issue Aug 22, 2024 · 7 comments

Comments

@fly-1011
Copy link

Bug Description

In some cases where an OF exception was triggered,NXin the fflags register was not set correctly.

Steps to Reproduce

.globl main
main:

    la t0, value_ft4        
    flw ft4, 0(t0)          
   
    la t1, value_ft6     
    flw ft6, 0(t1)          

    fdiv.s ft0,ft6,ft4
  
    csrr t2, fflags
.section .data
.align 4
value_ft4:
    .word 0x3f7fffff
.align 4
value_ft6:
    .word   0x7f7fffff

The log from CVA6 is as follows:

core   0: 0x0000000080002018 (0x18437053) fdiv.s  ft0, ft6, ft4
3 0x0000000080002018 (0x18437053) f 0 0xffffffff7f800000
core   0: 0x000000008000201c (0x001023f3) csrrs   t2, fflags, zero
3 0x000000008000201c (0x001023f3) x 7 0x0000000000000004

The log from Spike is as follows:

core   0: 0x0000000080002018 (0x18437053) fdiv.s  ft0, ft6, ft4
core   0: 3 0x0000000080002018 (0x18437053) c1_fflags 0x0000000000000005 f0  0xffffffff7f800000
core   0: 0x000000008000201c (0x001023f3) csrrs   t2, fflags, zero
core   0: 3 0x000000008000201c (0x001023f3) x7  0x0000000000000005

Note:

This issue does not require clearing the fflags register to be triggered. It is distinct from issue pulp-platform/fpu_div_sqrt_mvp#15, which has already been resolved. In my current environment, the previous issue is no longer reproducible.

Below is the cva6 log of the previous issue running in my environment:

core   0: 0x0000000080002014 (0x000e3107) fld     ft2, 0(t3)
3 0x0000000080002014 (0x000e3107) f 2 0x000000000828d569
core   0: 0x0000000080002018 (0x1a1172d3) fdiv.d  ft5, ft2, ft1
3 0x0000000080002018 (0x1a1172d3) f 5 0x8000000000000000
core   0: 0x000000008000201c (0x001023f3) csrrs   t2, fflags, zero
3 0x000000008000201c (0x001023f3) x 7 0x0000000000000003
@MikeOpenHWGroup
Copy link
Member

MikeOpenHWGroup commented Aug 22, 2024

Hi @fly-1011, thanks for this issue. It seems you are using the CVA6 (which is great!). However, the CVA6 uses a very out-of-date version of CVFPU:

commit 3116391bf66660f806b45e212b9949c528b4e270
Author: Luca Bertaccini <[email protected]>
Date:   Fri Mar 17 12:00:42 2023 +0100

    Release 0.7.0 (#80)

    Create release 0.7.0:

    Align CVFPU to RVV requirements (ARA branch merged)
    Fix f2i cast edge cases
    Fix RDN bug in floating-point multiplications
    Fix shift amount width in fma and fma_multi

There have been 41 newer commits made to CVFPU after 3116391.

As far as a I aware, there are no active CVA6 variants that support floating point, so there might not be much interest in resolving this issue among the CVA6 team for some time. (There are at least two CVA6 variants that will support F and/or D ISAs, but these have not yet started.)

Also, because 3116391 is so old, it is possible that this issue has already been fixed.

Do you have access to an environment that does not rely on the CVA6?

@fly-1011
Copy link
Author

Hi @fly-1011, thanks for this issue. It seems you are using the CVA6 (which is great!). However, the CVA6 uses a very out-of-date version of CVFPU:

commit 3116391bf66660f806b45e212b9949c528b4e270
Author: Luca Bertaccini <[email protected]>
Date:   Fri Mar 17 12:00:42 2023 +0100

    Release 0.7.0 (#80)

    Create release 0.7.0:

    Align CVFPU to RVV requirements (ARA branch merged)
    Fix f2i cast edge cases
    Fix RDN bug in floating-point multiplications
    Fix shift amount width in fma and fma_multi

There have been 41 newer commits made to CVFPU after 3116391.

As far as a I aware, there are no active CVA6 variants that support floating point, so there might not be much interest in resolving this issue among the CVA6 team for some time. (There are at least two CVA6 variants that will support F and/or D ISAs, but these have not yet started.)

Also, because 3116391 is so old, it is possible that this issue has already been fixed.

Do you have access to an environment that does not rely on the CVA6?

Thank you for your detailed explanation and for pointing out the issue with the CVFPU version. At the moment, I don't have access to an environment that doesn't rely on CVA6 for testing. However, based on my observations, this issue does indeed appear to be a bug in the version we have used.

I will further investigate this issue and look for possible solutions. If you have any other suggestions or recommended tools that could help me more effectively verify and resolve this issue, I would greatly appreciate it.

Thank you again for your support and assistance!

@MikeOpenHWGroup
Copy link
Member

The best place to discuss the CVA6's use of CVFPU is the OpenHW Group's Mattermost discussion board.

@MikeOpenHWGroup
Copy link
Member

Hi again @fly-1011, we have not forgotten this issue! 😉

As you may know, this IP was extensively verified in CV32E40P (v1.8.3) so it is more than a little surprising to see this Issue. However, within the CV32E40P context, the CVFPU was only verified with a very specific set of instantiation parameters. You can see these in the User Manual here. Can you list the instantiation parameters you are using in your testbench?

@fly-1011
Copy link
Author

fly-1011 commented Sep 6, 2024

Hi @MikeOpenHWGroup ,😉

Thanks for the follow-up! I am running my tests using the following command:

python3 cva6.py --target cv64a6_imafdc_sv39 --iss=$DV_SIMULATORS --iss_yaml=cva6.yaml --asm_tests ../tests/custom/hello_world/custom_test_template.S --linker=../tests/custom/common/test.ld --gcc_opts="-static -mcmodel=medany -fvisibility=hidden -nostdlib -nostartfiles -g ../tests/custom/common/syscalls.c ../tests/custom/common/crt.S -lgcc -I../tests/custom/env -I../tests/custom/common"

The commit I am using for CVA6 is 73590010e63e9740994fa15fbc5cd5fe85d24bb4. Please let me know if you need more specific details about the environment.

Thanks again for your help!

@pascalgouedo
Copy link

Same answer than #110

@fly-1011
Copy link
Author

fly-1011 commented Sep 9, 2024

Same answer than #110

Please see the answer in #120 (comment)

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

No branches or pull requests

3 participants