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

Zip and Unzip descriptions are reversed #1632

Closed
wmat opened this issue Sep 6, 2024 · 6 comments · Fixed by #1635
Closed

Zip and Unzip descriptions are reversed #1632

wmat opened this issue Sep 6, 2024 · 6 comments · Fixed by #1635

Comments

@wmat
Copy link
Collaborator

wmat commented Sep 6, 2024

As reported on reddit "In the standard page 267 the description of the zip instruction doesn't match the operation, which is correct?" by PColim in the risc-v subreddit. This applies to the 20240811 published version of unpriv.

@aswaterman
Copy link
Member

@kdockser I'm having deja vu; I thought we already resolved this.

It looks like the SAIL code is correct but the descriptions are swapped. Can you confirm and either file a PR or let me know so I can do so?

@kdockser
Copy link
Collaborator

kdockser commented Sep 7, 2024

As I recall, no one was sure how to modify and rebuild the scalar crypto spec, so fixing errors like this kept getting put off.
Please go ahead and file a PR for this (and any other outstanding fixes for Scalar crypto).
Thx

@pierrecolim
Copy link

Not only the description but also the encoding is incorrect, wording should be the same as in the ratified paper ( https://github.com/riscv/riscv-crypto/releases/download/v1.0.1-scalar/riscv-crypto-spec-scalar-v1.0.1.pdf )

@aswaterman
Copy link
Member

@pierrecolim could you file a PR?

@pierrecolim
Copy link

no sorry, i never used github, i only have an account to open issues and comment

aswaterman added a commit that referenced this issue Sep 9, 2024
Something went wrong when these specs were integrated into the main ISA
manual.  This PR restores the definitions of these instructions to the
ratified ones.

Resolves #1632
@aswaterman
Copy link
Member

OK, done. LMK if you spot any further issues @kdockser @pierrecolim #1635

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 a pull request may close this issue.

4 participants