Zfh's 'flh-align-01' test is executed even if Zfh is not enabled

[stnolting]

I am using the latest version of this repository for a custom RISCOF port. The test fails because the Zfh/src/flh-align-01.S test is executed although the Zfh ISA extension is not enabled.

• My test run of RISCOF: https://github.com/stnolting/neorv32-riscof/actions/runs/8446548698/job/23135504941#step:8:234
• The port's ISA config: https://github.com/stnolting/neorv32-riscof/blob/main/plugin-neorv32/neorv32_isa.yaml#L3

I am not sure, but maybe there is a typo in the flh-align test's regex list:

riscv-arch-test/riscv-test-suite/rv32i_m/Zfh/src/flh-align-01.S

Line 32 in 8a0cdce

RVTEST_CASE(0,"//check ISA:=regex(.*I.*);def TEST_CASE_1=True;",flh-align)

which (I think) should also include F and Zfh as in the remaining Zfh test cases:

riscv-arch-test/riscv-test-suite/rv32i_m/Zfh/src/flt_b1-01.S

Line 32 in 8a0cdce

RVTEST_CASE(0,"//check ISA:=regex(.*I.*F.*Zfh.*);def TEST_CASE_1=True;",flt_b1)

[davidharrishmc]

flh_align-01.S contains

RVTEST_ISA("RV32IF_Zicsr_Zfh,RV64IF_Zicsr_Zfh")

When I build the tests with riscof, this prevents running this test on a system without Zfh.

If you are building the tests with a different process that examines RVTEST_CASE rather than RVTEST_ISA, maybe the RVTEST_CASE should be more complete. It should be harmless to make RVTEST_CASE more comprehensive.

I'd suggest you make a PR with the change. If it's hard for you to make this PR, I can make it for you.

[allenjbaum]

It seems to me that every test in the Zfh directory has exactly the same RISC_ISA parameter, and exactly the same RVTEST_CASE parameters, so either all should fail or none should fail. Is this not the case, but just flh_align is failing? That seems a bit strange, and if that is the case, we need to figure out why before we start changing anything.

If there is some shortcoming/bug in riscof that requires that TEST_CASE change - that's one thing,
But I'm not very inclined to make changes to tests for someone's custom implementation unless there is some underlying issue that the current tests are failing,
Since it doesn't seem to be failing for others, that points to your custom implementation being the problem.

The alternative is to make sure that your custom riscof at least follow the rules that tests should meet both RISC_ISA rules and TEST_CASE rules.

Are any of my assertions here mistaken?

[davidharrishmc]

The mistaken assertion is that not all tests have the same RISCV_ISA and RVTEST_CASE parameters. flh is an outlier for RISCV_TEST_CASE, which is causing the problem.

flh-align-01 has

RVTEST_ISA("RV32IF_Zicsr_Zfh,RV64IF_Zicsr_Zfh")
...
RVTEST_CASE(0,"//check ISA:=regex(.*I.*);def TEST_CASE_1=True;",flh-align)

while other test cases such as flt_b1-01.S has

RVTEST_ISA("RV32IF_Zicsr_Zfh,RV64IF_Zicsr_Zfh")
...
RVTEST_CASE(0,"//check ISA:=regex(.*I.*F.*Zfh.*);def TEST_CASE_1=True;",flt_b1)

[allenjbaum]

Huh; I thought I looked at the other tests and they were the same.
And I misunderstood David's comment - I thought he was saying something else was going on.
File the PR for this change (one parameter in the RVTEST_CASE macro) , and we will try to get that done quickly.
I think we need that done on the dev branch (and we will probably need to ensure that all other PRs outstanding on the main Branch get moved to the dev branch.

[stnolting]

I'll do a PR (#451).
Zfh is not yet available on the dev branch so I'll make a PR targeting the main branch.

[allenjbaum]

We clearly need a cleanup on the dev branch & start doing release from the dev branch, so no more PRs to the main branch. There's too much stuff in flight right now.

…

On Tue, Apr 9, 2024 at 7:19 AM stnolting ***@***.***> wrote: I'll do a PR. Zfh is not yet available on the dev branch so I'll make a PR targeting the main branch. — Reply to this email directly, view it on GitHub <#448 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJWBZ3ASITXNKPAQNO3Y4P2GZAVCNFSM6AAAAABFLFXABSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBVGI4TONRWGM> . You are receiving this because you commented.Message ID: ***@***.***>