riscv-non-isa/riscv-arch-test

The FPU tests do not compile if rvtest_mtrap_routine is defined

algrobman opened this issue · 6 comments

tsig_[begin|end]_canary is mentioned twice in the FP tests sources:

see ex:

#ifdef rvtest_mtrap_routine

#ifdef rvtest_mtrap_routine

tsig_begin_canary:
CANARY;
tsig_begin_canary:
CANARY;
mtrap_sigptr:
.fill 64*(XLEN/32),4,0xdeadbeef
tsig_end_canary:
CANARY;
tsig_end_canary:
CANARY;

#endif

It make hard to run them outside riscof , please, fix this in next opportunity

Unclear why that wasn't caught in CI - or why it matters whether it is running under riscof or not.

RISCOF provides this rvtest_mtrap_routine define as part of run command, I guess for only tests, which suppose to have exceptions .

We had set this define in model_test.h and it worked for all integer tests, until we added FP tests ...

I'm not sure that this is something we need to fix. The tests themselves are supposed to set it.
Why are you setting it? What breaks if you don't?
Going a bit further: the tests for architectural compatibility are intended to be run under the riscof framework.
Riscof looks at the test_case macro parameters in the test to decide whether to install the trap handler

If you need to get branding, then you need to run under riscof - so no changes are needed
If you don't need to get branding, there is no need to run the ACTs - so no changes are needed
I see no rationale for us to make changes in order to make these tests run independently of riscof;
A simpler solution is to modify your model_test.h to avoid setting it when you run FP tests.
There is no guarantee you won't run into similar problems with test for other extensions, of course.

too many words of excuses instead of just fixing plain incorrect code ...

we use these tests in our verification environment which does not have riscof automatization - so we had to patch their sources.
I guess others will eventually face the same problem