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

Add Smctr/Sscctr #1615

Merged
merged 5 commits into from
Nov 21, 2024
Merged

Add Smctr/Sscctr #1615

merged 5 commits into from
Nov 21, 2024

Conversation

bcstrongx
Copy link
Collaborator

Not yet ratified, going to TSC vote soon.

@bcstrongx bcstrongx requested a review from wmat August 28, 2024 14:52
Copy link
Collaborator

@wmat wmat left a comment

Choose a reason for hiding this comment

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

The two changes here LGTM, however, don't forget to add the include::smctr.adoc to riscv-privileged.adoc.

@bcstrongx
Copy link
Collaborator Author

Good catch! Added.

Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

Is it really 1.0 given that it hasn't been ratified? I think the options are to leave the PR unmerged until ratification, or to decrement the version number and add the Frozen warning.

Also, don't forget to update the extension list in the preface.

@bcstrongx
Copy link
Collaborator Author

Definitely not 1.0 yet. I assumed this PR was going to sit around until the extension is ratified, so v1.0 was appropriate. Do we normally merge frozen but not ratified extensions into the ISA manual? If so I can modify this.

@aswaterman
Copy link
Member

I think either way is OK. Leaving this PR alone until ratification is the minimal-effort thing, so if you're OK with that, let's do that.

@bcstrongx
Copy link
Collaborator Author

I updated the preface, again as though the extensions are ratified. We'll wait until ratification to merge.

@bcstrongx
Copy link
Collaborator Author

I believe this PR is ready to merge. CTR is ratified (v1.0), and I pushed some late clarifications that were added to the CTR repo. I removed any mention of CTR from the preface, since those entries seem to be made when a new release is released. I don't know when that will be, so I assume @wmat will create that. @aswaterman you had a change request related to this, please either approve or let me know if a change is still needed.

Copy link
Member

@aswaterman aswaterman left a comment

Choose a reason for hiding this comment

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

Thanks, @bcstrongx!

@aswaterman aswaterman merged commit d1f83a7 into main Nov 21, 2024
2 checks passed
@aswaterman aswaterman deleted the dev/beeman/smctr-ssctr branch November 21, 2024 23:59
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