Add support for the unratified Zalasr extension

[mehnadnerd]

Adds support for the Zalasr extension. Documentation on that is available at https://github.com/riscv/riscv-zalasr

[arichardson]

The new file is a slightly modifed copy of the (old version of) LOAD/STORE, can't you call those functions instead and make the alignment check mandatory (since al==true) rather than dependent on plat_enable_misaligned_access()?

[mehnadnerd]

I tried to match what was done in the A extension (for load-reserved and store-conditional). Should I change this to use LOAD/STORE (and add a new parameter to force alignment checks) instead?

[github-actions[bot]]

Test Results

398 tests  +2   398 ✅ +2   1m 19s ⏱️ -1s   1 suites ±0     0 💤 ±0    1 files   ±0     0 ❌ ±0 

Results for commit f5d75c8c. ± Comparison against base commit 91da83ee.

:recycle: This comment has been updated with latest results.

[mehnadnerd]

I've upgraded it to be based on the most recent load and store invocations. I added tests from https://github.com/riscv-software-src/riscv-tests/pull/562.

Spec is now at https://github.com/riscv/riscv-zalasr

[Timmmm]

I think we also shouldn't be checking in binary ELFs anymore either. We finally have some infrastructure for first party tests but I guess if we want something from newer versions of riscv-tests we should update it (without checking in ELFs). It's on the roadmap.

[mehnadnerd]

I think we also shouldn't be checking in binary ELFs anymore either. We finally have some infrastructure for first party tests but I guess if we want something from newer versions of riscv-tests we should update it (without checking in ELFs). It's on the roadmap.

Are you sure? This would result in Zalasr being untested in the checkin. This already caught an issue where I didn't upgrade the cmakelists properly, only the Makefile.

[jordancarlin]

This PR brings up an interesting question about how to handle extensions that aren't ratified yet. I think that generally the golden model should not have features that are not officially in the spec yet. I can think of two possible approaches to this:

1. We keep unratified features as PRs and wait to merge them until the extension is ratified and we confirm the implementation matches the final version of the spec
2. We merge the extension but keep it behind an --enable-experimental flag (or something similar) so that it is disabled and requires users to explicitly opt in to the not yet finalized extensions. Once the extensions is ratified, we review our implementation and remove the flag.

[jordancarlin]

I think we also shouldn't be checking in binary ELFs anymore either. We finally have some infrastructure for first party tests but I guess if we want something from newer versions of riscv-tests we should update it (without checking in ELFs). It's on the roadmap.

Are you sure? This would result in Zalasr being untested in the checkin. This already caught an issue where I didn't upgrade the cmakelists properly, only the Makefile.

Is it possible to check in an assembly or C file that can be used for testing instead? Or is support for this extension not yet in upstream compilers.

Also, the extension needs to be added to the list of supported extensions in the README.

[mehnadnerd]

I think we also shouldn't be checking in binary ELFs anymore either. We finally have some infrastructure for first party tests but I guess if we want something from newer versions of riscv-tests we should update it (without checking in ELFs). It's on the roadmap.

Are you sure? This would result in Zalasr being untested in the checkin. This already caught an issue where I didn't upgrade the cmakelists properly, only the Makefile.

Is it possible to check in an assembly or C file that can be used for testing instead? Or is support for this extension not yet in upstream compilers.

Also, the extension needs to be added to the list of supported extensions in the README.

We could check in the test files I introduce in https://github.com/riscv-software-src/riscv-tests/pull/562 (which these are derived from), although you need a recent version of LLVM (19 or later) for asm support. This would also be contrary to what the other tests in that directory do (they are raw elf files).

I've added the note in the README.

This PR brings up an interesting question about how to handle extensions that aren't ratified yet. I think that generally the golden model should not have features that are not officially in the spec yet. I can think of two possible approaches to this:

1. We keep unratified features as PRs and wait to merge them until the extension is ratified and we confirm the implementation matches the final version of the spec

2. We merge the extension but keep it behind an `--enable-experimental` flag (or something similar) so that it is disabled and requires users to explicitly opt in to the not yet finalized extensions. Once the extensions is ratified, we review our implementation and remove the flag.

The easiest thing to do is probably to hold off on merging until it is ratified, if you would like to do so let me know.

[jordancarlin]

@mehnadnerd Now that the --enable-experimental-extensions flag has been added to the model (see #800), we can look at merging this if you update it to be gated on that flag.

[Timmmm]

I also think we should update the riscv-tests to build from source and then we can update to the latest version. If it requires a recent Clang, so be it. It's easy to install.

[mehnadnerd]

I think i'm going to wait for the extension to be officially approval to try to merge, so we don't have to do the dance of merging it twice, once now and once when it is approved.

[jordancarlin]

Looks like this extension has been officially approved by the TSC and is just pending board approval for ratification. It would be good to get this updated and merged.

[Timmmm]

I'd quite like to get #1262 and then #792 merged first. #1262 could do with a review. It does remove some enums that will be needed in this PR but we can add them back here.