rv32i/rv32mi has tests incompatible with implementation supporting compressed about riscv-arch-test HOT 9 CLOSED

Test MISALING_JMP was intended to run only for configurations without compressed instructions.
I think there is also option 3 - to use "ifdef" in the asm file to skip problematic parts. But for some specific tests it may be useless (if it is used to skip all test, it is useless to run it).

While defines are not supported now in the framework I would prefer option number 1. I would not use number 2 solution because user can have user specific compressed extension while not supporting C ISA extension. Test will check for C extension (which is not present) and fail.

from riscv-arch-test.

In theory, there should be a parameter passed to framework that indicates whether compressed is supported or not, and based on that the environ should include or skip tests that either check or use conpressed ops (and halfword aligned ops). One reason for this is that tests that check if compressed support is present require access of MISA CSR (or an ECALL that returns the answer).

Both of these require execution of a privileged op or access of a privileged CSR, and these tests are specifically intended to check the unprivileged ISA and to not depend on ANY privileged op or resource.

from riscv-arch-test.

@hajekra
The compliance suite is aimed at testing the RISC-V standard extensions and not user defined extensions.
If an implementation supports a non-standard compressed ISA while not supporting C ISA, then passing this test "as is" would mean it is standard "C" compliant which is not true.
I believe the compliance suite is aimed at only testing standard RISC-V ISA compliance and not user extension compliance?

Having a simple check on whether "C" ISA is supported or not should work for both these tests.
Please note however, MISA is an optional CSR and cannot be used to check for support of C extensions. Maybe a forced trap mechanism as a prologue can be used to check if "C" is supported.

from riscv-arch-test.

@neelgala
My understanding is:
Compliance suite is only for standard extensions (defined in spec), but it should be usable also for user extended implementations (but will test only standard extensions). Passing one test does not mean implementation is compliant, all tests have to pass. So to be compliant with C extension also all ISA tests have to pass (all C instructions have to be implemented). So basically user can have RV32I with only part of the C extension implemented. In this case "C" is user defined extension and this particular test should not be used (there is user defined 16-bit extension), but it can still be RV32I compliant.

Could you please provide example what you mean by forced trap? Maybe I got it wrong but in my opinion this would lead to following execution:
Try misalign trap ->
a) it is trap, continue to test = it is working, test it
b) it is not trap, skip the tests = there is C or it is not working, skip it

from riscv-arch-test.

Regarding the prolog, I think the MISA register should be checked for supported extensions at the start of the test, in order to determine if the HW has the required feature set to run the test.
If the MISA returns 0 (meaning un-implemented) then I think we should go ahead and run the test, as if it were supported.
Where the MISA is not implemented it will need to be the device creator to decide upon the set of tests to be executed or provide a non-standard mechanism to determine the capabilities.

from riscv-arch-test.

@hajekra
Yes your trap routine flow is what I had in mind. While this is what ma_fetch in rv32mi suite does it still has a bug which generates a different signature for devices with and without "C" support.
line 202 of ma_fetch
addi a1, a1, 12
should be
addi a1, a1, 8
Will do a pull request for this or you could do the fix as well. This will generate the same signature in either case.

@eroom1966
What you say could work, but there is another catch here, implementations which implement MISA need not allow you to "switch off" certain extensions. This is valid by the spec. MISA being a WARL register can have certain bits to be readonly.
Consider the I-MISALIGN_JMP-01.S test in rv32i suite. The test tries to switch off "C" through misa before running the test. This will not work in implementations having "C" bit as readonly in MISA. The issue is in how the trap_handler is written.

I have figured out the issues/bugs in both ma_fetch and I-MISALIGN_JMP-01.S and would like to create separate issues for each to track them. I would like to close this issue if you guys agree?

from riscv-arch-test.

from riscv-arch-test.

@neelgala
Ok, let's create new issues and close this one. By the way, I am porting test to Codasip framework which is capable of selecting tests by given configuration and setting. Right now I skip both of them when C extension is implemented.

@allenjbaum
I totally agree that tests should be skipped by given configuration and that is what we try to do. But in case of I-MISALIGN_JMP-01.S test the question is if it should be part of the ISA tests. In user-level ISA spec is:

The JAL and JALR instructions will generate a misaligned instruction fetch exception if the target
address is not aligned to a four-byte boundary.

So it is part of user-level spec which should be tested by ISA tests but on the other hand it is using privileged mechanism (trap).

from riscv-arch-test.

created #15 and #16
Closing this issue.

from riscv-arch-test.