Add basic jal misalign test

[trdthg]

Added a list tests_fail to specify which test should fail.

Also added two very basic jal tests to verify that an error occurs when target address misaligned.

[github-actions]

Test Results

400 tests  +4   400 ✅ +4   1m 19s ⏱️ ±0s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 62c2ab6. ± Comparison against base commit 4f3907e.

[Timmmm]

I think it would be nicer just to make the tests pass, but check the negative condition, i.e. install a custom trap handler temporarily, have it record if there was a misaligned exception and return, then uninstall it.

[trdthg]

I think it would be nicer just to make the tests pass, but check the negative condition, i.e. install a custom trap handler temporarily, have it record if there was a misaligned exception and return, then uninstall it.

Good idea, this is more precise. I tried to update a bit, but it currently can't pass the test

Here are some log

mem[X,0x000000008000200C] -> 0x3052
[114] [M]: 0x000000008000200A (0x30529073) csrrw zero, mtvec, t0
CSR mtvec <- 0x000000008000202C (input: 0x000000008000202E)
mem[X,0x000000008000200E] -> 0x4F01
[115] [M]: 0x000000008000200E (0x4F01) c.li t5, 0x0
x30 <- 0x0000000000000000
mem[X,0x0000000080002010] -> 0xA829
[116] [M]: 0x0000000080002010 (0xA829) c.j 0x1a
mem[X,0x000000008000202A] -> 0x0000
[117] [M]: 0x000000008000202A (0x0000) c.illegal 0x0
trapping from M to M to handle illegal-instruction
handling exc#0x02 at priv M with tval 0x0000000000000000
CSR mstatus <- 0x8000000A00007800
mem[X,0x000000008000202C] -> 0x0000
[118] [M]: 0x000000008000202C (0x0000) c.illegal 0x0

So this test may need to wait for further improvements in the test infrastructure

---

This test causes the CI to get stuck (in an infinite loop), don't know how GitHub will handle it 🆒

[Timmmm]

Yeah we need tests to be able to specify what config to use. I think we'll have to wait until the new config system is available, then we can embed some JSON in them as a comment or something.

[arichardson]

Not opposed to adding more tests, but I would hope this is also covered by any existing riscv-tests or riscv-arch-test? Can we try to run those tests in CI?

[Timmmm]

This doesn't make sense. You can't jump to an odd address. What are you trying to test here exactly?

[nadime15]

Maybe I'm missing something, but isn't that what he wants to test? He wants to cause a misaligned exception

[jordancarlin]

jal can never jump to an odd address though. The least significant bit of the offset isn't encoded in the immediate and is always 0. Misaligned exceptions with jal can only happen when compressed instructions are not supported. Then a jump to a 2 (but not 4) byte aligned word will cause an exception.

[nadime15]

Ok I thought either:

1. He explicitly wants to test if jal is correctly implemented by attempting a jump that should generate a trap if the processor correctly enforces alignment requirements, or

2. He wants to test a jump to an address with a 2-byte offset when the C extension is not supported. In this case, it should be .space 2 after the misalign: label, not .space 3. (I guess)

[trdthg]

jal can never jump to an odd address though. The least significant bit of the offset isn't encoded in the immediate and is always 0.

I forgot this. 🙏

Ok I thought either:

Both were the original idea.

[jordancarlin]

In that case, I imagine this test already exists in riscv-tests and/or riscv-arch-test?

[trdthg]

• For riscv-tests: I haven't found anything related to jal and misalign (I don't used riscv-tests much, someone'd better check it again)
• For riscv-arch-test: There are, but I don't know how it checks mcause. Does it just assert that the elf run failed?
  https://github.com/riscv-non-isa/riscv-arch-test/blob/752c0fb721b9ab264682d491283d98813f95e45b/coverage/cgf.yaml#L422-L430

[jordancarlin]

Looks like the relevant test in riscv-arch-test is here: https://github.com/riscv-non-isa/riscv-arch-test/blob/752c0fb721b9ab264682d491283d98813f95e45b/riscv-test-suite/rv32i_m/privilege/src/misalign-jal-01.S.

When it traps, the riscv-arch-test trap handler stores all of the relevant CSRs (including mcause) to the signature region of memory, and they are compared to another model (usually Spike).

The cvw-arch-verif tests that are going to be integrated into riscv-arch-test also test misaligned jal: https://github.com/openhwgroup/cvw-arch-verif/blob/f13040a0b752281e3e08f9076d8d164d955a4493/tests/lockstep/priv/ExceptionsM.S#L81

[pmundkur]

This test is also in riscv-tests:
https://github.com/riscv-software-src/riscv-tests/blob/master/isa/rv64si/ma_fetch.S

[Timmmm]

riscv-tests are pretty easy and already in there; we just need to update them. The problem with riscv-tests is it's extremely basic and it assumes a fixed ISA where basically everything is supported, so it can't do something like testing for misaligned instruction exceptions.

riscv-arch-tests does have RISCOF which you can tell the ISA string and it will select tests based on that, but it's also pretty basic and RISCOF is kind of a mess. Still, I do think we probably should get the riscv-arch-tests running in CI if we can. Probably worth the hassle. I would maybe say we just skip RISCOF entirely and use the assembly directly though. Probably less painful.

[jordancarlin]

riscv-tests are pretty easy and already in there; we just need to update them. The problem with riscv-tests is it's extremely basic and it assumes a fixed ISA where basically everything is supported, so it can't do something like testing for misaligned instruction exceptions.

Now that we require a compiler for some of the tests (and none of the tests are needed by end users), I wonder if we should switch to using the riscv-tests from source instead of keeping the binaries in the repo. That would also make it easier to keep up to date.

riscv-arch-tests does have RISCOF which you can tell the ISA string and it will select tests based on that, but it's also pretty basic and RISCOF is kind of a mess. Still, I do think we probably should get the riscv-arch-tests running in CI if we can. Probably worth the hassle. I would maybe say we just skip RISCOF entirely and use the assembly directly though. Probably less painful.

riscv-arch-test is considering a fairly significant overhaul in the very near future, so it probably makes sense to wait and see where things land with that first.

[nadime15]

Any opinions on including the vector test suite as a submodule or subtree in the repo (perhaps as an optional test suite)? We had a discussion about it today in the dev partners meeting and I know ACT wants to add it at some point.

[jordancarlin]

Any opinions on including the vector test suite as a submodule or subtree in the repo (perhaps as an optional test suite)? We had a discussion about it today in the dev partners meeting and I know ACT wants to add it at some point.

Does it make sense to wait and use it directly from riscv-arch-test once it is integrated if that is the long term plan?

[Timmmm]

Now that we require a compiler for some of the tests (and none of the tests are needed by end users), I wonder if we should switch to using the riscv-tests from source instead of keeping the binaries in the repo. That would also make it easier to keep up to date.

Seems like a good idea. Technically you can still develop without running the first_party tests and then rely on CI for riscv-tests... but I think given how easy it is to install Clang (even if it is huge) it's probably ok to require that.

riscv-arch-test is considering a fairly significant overhaul in the very near future

Ah interesting, any idea what?

[nadime15]

@jordancarlin Yes, I think it makes sense. There is no rush anyway.

[arichardson]

riscv-tests are pretty easy and already in there; we just need to update them. The problem with riscv-tests is it's extremely basic and it assumes a fixed ISA where basically everything is supported, so it can't do something like testing for misaligned instruction exceptions.

riscv-arch-tests does have RISCOF which you can tell the ISA string and it will select tests based on that, but it's also pretty basic and RISCOF is kind of a mess. Still, I do think we probably should get the riscv-arch-tests running in CI if we can. Probably worth the hassle. I would maybe say we just skip RISCOF entirely and use the assembly directly though. Probably less painful.

Fully agree with all your statements. I do think CI should compile riscv-tests from source now that we have the infra for it instead of using pre-compiled binaries. While it assumes a fixed ISA, we should be able to support that expected ISA config once the config file support lands.

[jordancarlin]

Technically you can still develop without running the first_party tests and then rely on CI for riscv-tests... but I think given how easy it is to install Clang (even if it is huge) it's probably ok to require that.

That is a good point, but I think the benefits of not storing elfs in the repo and the reduced work to keep up to date with riscv-tests probably outweigh the cost of requiring the compiler. I imagine most people who are developing will have a compiler installed already (especially if we actually start asking for tests for new features).

riscv-arch-test is considering a fairly significant overhaul in the very near future

Ah interesting, any idea what?

riscv-arch-test and cvw-arch-verif are likely going to be merging to some degree. The exact details are still being figured out, but at a minimum the privileged tests from cvw-arch-verif are going to migrate to riscv-arch-test, and there are discussions of replacing riscv-ctg and riscv-isac with SystemVerilog coverpoints from cvw-arch-verif as well. If things go down the second route, then many of the unprivileged tests will be updated as well.

[arichardson]

Technically you can still develop without running the first_party tests and then rely on CI for riscv-tests... but I think given how easy it is to install Clang (even if it is huge) it's probably ok to require that.

That is a good point, but I think the benefits of not storing elfs in the repo and the reduced work to keep up to date with riscv-tests probably outweigh the cost of requiring the compiler. I imagine most people who are developing will have a compiler installed already (especially if we actually start asking for tests for new features).

As long as it's still possible to build the model without needing a cross-compiler I don't see any problem. It's easy to gate building and running the tests with a -DENABLE_TESTS=ON/OFF flag that users who only want to build sail as a simulator/for formal proofs/for documentation generation/etc. can set.

[trdthg]

We already have the riscv-tests，first-party test, and we will have riscv-arch-test (based on riscv-admin/dev-partners#38)

From my understanding, the relationship between these tests is

• The development of new tests should better be based on riscv-arch-test
• For tests that can't be handled by riscof, or sail specific test (anyone could give me an example?) will be based on first-party test
• riscv-tests serves only as a basic guarantee and should be continuously updated with upstream

If I understand correctly, this PR should be closed now?

[jordancarlin]

Seems like a similar test is already in both riscv-arch-test and riscv-tests, so no reason to add this as a first-party test. @trdthg are you ok with closing this?

[trdthg]

Of course