[BUG] NX flag not correctly triggered in specific cases

fly-1011 commented 2 months ago

Bug Description

In some cases where an OFexception 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 https://github.com/pulp-platform/fpu_div_sqrt_mvp/issues/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 commented 2 months ago

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 <55843305+lucabertaccini@users.noreply.github.com>
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 commented 2 months ago

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 <55843305+lucabertaccini@users.noreply.github.com>
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 commented 2 months ago

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

MikeOpenHWGroup commented 1 month ago

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

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 commented 1 month ago

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 commented 1 month ago

Same answer than #110

fly-1011 commented 1 month ago

Same answer than #110

Please see the answer in https://github.com/openhwgroup/cvfpu/issues/120#issuecomment-2337965138

openhwgroup / cvfpu