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

crypto/elliptic: s390x elliptic assembly refactoring in Go1.19 [freeze exception] #52709

Closed
billotosyr opened this issue May 4, 2022 · 6 comments
Closed
Labels
arch-s390x NeedsFix
Milestone

Comments

@billotosyr
Copy link

@billotosyr billotosyr commented May 4, 2022

We are working to move the s390x elliptic assembly into the new refactored form as designated by Filippo, but we need more time to get everything reviewed and finalized. We have been working to base ours against Filippos CLs, (not all of which have been integrated). For now we are basing our changes on CL 399755, as suggested by Filippo. Given the new +2 review policy, it will be a challenge to meet the freeze deadline, but it shouldn't spill out much past the deadline. Note that this only affects s390x linux -- no other platform. I respectfully request a reasonable grace period beyond the freeze. Our CL has begun the review process: CL 404058

@l-lindsay
Copy link

@l-lindsay l-lindsay commented May 4, 2022

@FiloSottile FiloSottile changed the title crypto/elliptic s390x elliptic assembly refactoring in Go1.19 [freeze exception] crypto/elliptic: s390x elliptic assembly refactoring in Go1.19 [freeze exception] May 5, 2022
@FiloSottile FiloSottile added this to the Go1.19 milestone May 5, 2022
@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented May 5, 2022

I'm not opposed to a freeze exception on my side, as long as there are only s390x changes, like in CL 404058 currently. Note that even with the new review policy, the only major step is getting a single +2 from someone who can review the assembly changes. We'll take care of the other +1.

@dr2chase dr2chase added the NeedsFix label May 5, 2022
@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 5, 2022

marked it needs-fix to get it off the triage list (and because it's not quite done).

@FiloSottile
Copy link
Contributor

@FiloSottile FiloSottile commented May 17, 2022

Was this approved @golang/release? The CL is ready.

@gopherbot
Copy link

@gopherbot gopherbot commented May 17, 2022

Change https://go.dev/cl/404058 mentions this issue: crypto/internal/nistec: re-enable s390x asm for P-256

@cagedmantis
Copy link
Contributor

@cagedmantis cagedmantis commented May 17, 2022

This freeze exception has been approved.

@dmitshur dmitshur added the arch-s390x label May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-s390x NeedsFix
Projects
None yet
Development

No branches or pull requests

7 participants