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

Allow CSRRS/C to read CLEN-wide values #108

Merged
merged 4 commits into from
Feb 14, 2024
Merged

Conversation

andresag01
Copy link
Collaborator

Fixes #92

@andresag01
Copy link
Collaborator Author

andresag01 commented Feb 12, 2024

@PeterRugg: I tried this fix as proposed here.

@PeterRugg
Copy link
Contributor

Thanks for sorting this! Looks good, and the text is a lot clearer than the previous text. Is it worth adding a note about no write happening, and thus no representable/sealed check happening, when the rs1 index is zero (as discussed in the issue)? It's surely the most sensible interpretation at the moment, but could be worth making it explicit?

@andresag01
Copy link
Collaborator Author

@tariqkurd-repo @PeterRugg : I've made an explicit note about the no-write on CSRRS/CSRRC when rs1=x0

src/riscv-integration.adoc Outdated Show resolved Hide resolved
src/riscv-integration.adoc Show resolved Hide resolved
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.

This LGTM. Would be nice to have a note on csrr but not necessarily required in this pr.

@arichardson
Copy link
Collaborator

This LGTM. Would be nice to have a note on csrr but not necessarily required in this pr.

@andresag01 just to clarify this, I'm happy for you to rebase-and-merge this now. The note can be added later.

src/insns/csrr_32bit.adoc Show resolved Hide resolved
src/insns/csrr_32bit.adoc Outdated Show resolved Hide resolved
src/insns/csrr_32bit.adoc Outdated Show resolved Hide resolved
@andresag01
Copy link
Collaborator Author

@Timmmm : Could you please review again and resolve the conversation if you are happy with the changes? Thanks!

@tariqkurd-repo
Copy link
Collaborator

any objections to merging this one? @arichardson maybe you can do the honours

@arichardson arichardson merged commit 1969ae0 into riscv:main Feb 14, 2024
3 checks passed
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.

No way to achieve CSRR on a capability
5 participants