riscv / sail-riscv

Sail RISC-V model
https://lists.riscv.org/g/tech-golden-model
Other
467 stars 169 forks source link

Make CSR definitions scattered #543

Closed Timmmm closed 2 months ago

Timmmm commented 2 months ago

Resolve the strange split between read_CSR (which is not scattered) and ext_read_CSR (which is). Same for write_CSR and is_CSR_defined.

I changed the return type of read_CSR from option(xlenbits) to xlenbits since the code must already check is_CSR_defined before calling read_CSR. The only function that returned None() was the seed CSR in write_seed_csr, which actually meant you would get a weird "Unhandled write to CSR" if you wrote to seed.

I renamed & reordered the files slightly to make the scattered mapping work, but I haven't moved any of the actual definitions yet. In future we should actually scatter them.

Fixes #410

github-actions[bot] commented 2 months ago

Test Results

396 tests  ±0   396 :white_check_mark: ±0   0s :stopwatch: ±0s   4 suites ±0     0 :zzz: ±0    1 files   ±0     0 :x: ±0 

Results for commit 73f7467a. ± Comparison against base commit 07a5036d.

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

billmcspadden-riscv commented 2 months ago

Minstret and timers are overkill.

On Fri, Sep 13, 2024, 8:50 AM Alexander Richardson @.***> wrote:

@.**** commented on this pull request.

In model/riscv_insts_zicsr.sail https://github.com/riscv/sail-riscv/pull/543#discussion_r1759103836:

 if isWrite then {

let new_val : xlenbits = match op { CSRRW => rs1_val, CSRRS => csr_val | rs1_val, CSRRC => csr_val & ~(rs1_val) };

  • writeCSR(csr, new_val)
  • let final_val = write_CSR(csr, new_val);

I meant I am not sure if we want every implicit read/write of a CSR to be emitted in the logfile since that could end up being really verbose.

— Reply to this email directly, view it on GitHub https://github.com/riscv/sail-riscv/pull/543#discussion_r1759103836, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXROLOHWPUKOWYII3TLDRS3ZWMCVLAVCNFSM6AAAAABN4B2SIKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDGMBTGQ4DGNBYHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

Timmmm commented 2 months ago

I'll merge this in a few days if nobody objects. I think it's uncontroversial.