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

Implement Zcb extension #427

Merged
merged 1 commit into from
Apr 15, 2024
Merged

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Mar 6, 2024

This adds an implementation of the Zcb code size extension.

This fixes all of the comments on the previous PR #322 - and also adds flags to the emulator to enable the extension (it is disabled by default).

@Timmmm Timmmm mentioned this pull request Mar 6, 2024
Copy link

github-actions bot commented Mar 6, 2024

Test Results

712 tests  ±0   712 ✅ ±0   0s ⏱️ ±0s
  6 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit 910f90b. ± Comparison against base commit a2ffa85.

♻️ This comment has been updated with latest results.

@jjscheel jjscheel mentioned this pull request Mar 19, 2024
13 tasks
Copy link
Collaborator

@arichardson arichardson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have not checked the encodings are correct, but the code LGTM.
I'm not sure we actually need this command line flag (especially if you're not actually using it).

model/riscv_insts_zcb.sail Outdated Show resolved Hide resolved
This adds an implementation of the Zcb code size extension.

Co-authored-by: Martin Berger <martinberger@users.noreply.github.com>
@Timmmm
Copy link
Collaborator Author

Timmmm commented Apr 3, 2024

I have not checked the encodings are correct, but the code LGTM. I'm not sure we actually need this command line flag (especially if you're not actually using it).

I mainly added it for other people to use and to reduce the merge conflicts for us. I think it's worth keeping for when/if we eventually actually use riscv-config. Though I'm happy to be overruled if it would block this being merged.

@arichardson
Copy link
Collaborator

I have not checked the encodings are correct, but the code LGTM. I'm not sure we actually need this command line flag (especially if you're not actually using it).

I mainly added it for other people to use and to reduce the merge conflicts for us. I think it's worth keeping for when/if we eventually actually use riscv-config. Though I'm happy to be overruled if it would block this being merged.

I'm guessing a lot of this command line parsing will go away eventually, so if it makes things easier for you merging as is sounds good.

Copy link
Collaborator

@billmcspadden-riscv billmcspadden-riscv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@billmcspadden-riscv billmcspadden-riscv merged commit 16077e1 into riscv:master Apr 15, 2024
2 checks passed
@Timmmm Timmmm deleted the user/timh/zcb branch May 15, 2024 08:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants