Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify some base instruction execute clauses #713

Merged
merged 2 commits into from
Feb 17, 2025

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Feb 6, 2025

Refactor some of the base execute clauses to remove intermediate variables and unnecessary ifs. The motivation for this is to make them read a bit better when included in the ISA manual.

  1. Make shift_right_arith generic so you don't need a 32/64-bit version.
  2. Use rs1_val in fewer places - it's actually longer than X(rs1) anyway.
  3. Directly assign to X(rd) where possible.
  4. Avoid single letter variables.

@Timmmm Timmmm force-pushed the user/timh/snippet_refactor branch from d0317a2 to 0127442 Compare February 6, 2025 11:08
Copy link

github-actions bot commented Feb 6, 2025

Test Results

392 tests  ±0   392 ✅ ±0   1m 21s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 9d1b7bc. ± Comparison against base commit c5055f9.

♻️ This comment has been updated with latest results.

model/riscv_insts_base.sail Show resolved Hide resolved
Refactor some of the base `execute` clauses to remove intermediate variables and unnecessary `if`s. The motivation for this is to make them read a bit better when included in the ISA manual.

1. Make `shift_right_arith` generic so you don't need a 32/64-bit version.
2. Use `rs1_val` in fewer places - it's actually longer than `X(rs1)` anyway.
3. Directly assign to `X(rd)` where possible.
4. Avoid single letter variables.
I think this makes it a bit clearer, rather than having a random `+ 2` in there. If we had half-open intervals then it would be even nicer: `[log2_xlen >.. 0]`.
@Timmmm Timmmm force-pushed the user/timh/snippet_refactor branch from 0127442 to 9d1b7bc Compare February 13, 2025 11:56
@Timmmm
Copy link
Collaborator Author

Timmmm commented Feb 13, 2025

I added log2_xlen - I think [log2_xlen - 1 .. 0] is much clearer than [log2_xlen_bytes + 2 .. 0] (but I still really wish we could do [log2_xlen >.. 0]).

@Timmmm Timmmm added the will be merged Scheduled to be merged in a few days if nobody objects label Feb 14, 2025
@Timmmm Timmmm merged commit 14172f9 into riscv:master Feb 17, 2025
2 checks passed
@Timmmm Timmmm deleted the user/timh/snippet_refactor branch February 17, 2025 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will be merged Scheduled to be merged in a few days if nobody objects
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants