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

Stabilize AArch64 SHA3 intrinsics #1552

Merged

Conversation

tarcieri
Copy link
Contributor

@rustbot
Copy link
Collaborator

rustbot commented Mar 13, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Amanieu (or someone else) some time within the next two weeks.

@Amanieu
Copy link
Member

Amanieu commented Mar 13, 2024

These intrinsics are tested against clang and are known to produce the same results.

@rfcbot fcp merge

@Amanieu
Copy link
Member

Amanieu commented Mar 13, 2024

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Mar 13, 2024

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@dtolnay
Copy link
Member

dtolnay commented Mar 14, 2024

@rfcbot reviewed

@BurntSushi
Copy link
Member

BurntSushi commented Mar 30, 2024

The checkboxes are greyed out for me (I can't tick my box), but I approve.

@Amanieu
Copy link
Member

Amanieu commented Mar 30, 2024

I've ticked your box.

@rfcbot
Copy link

rfcbot commented Mar 30, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @Amanieu, I wasn't able to add the final-comment-period label, please do so.

@tarcieri
Copy link
Contributor Author

Merging master caused some build failures which seem to be unrelated:

https://github.com/rust-lang/stdarch/actions/runs/8491609619/job/23263755908?pr=1552#step:14:8910

   Compiling intrinsic-test v0.1.0 (/checkout/crates/intrinsic-test)
error: field `reg` is never read
  --> crates/intrinsic-test/src/json_parser.rs:21:9
   |
19 |     Register {
   |     -------- field in this variant
20 |         #[serde(rename = "register")]
21 |         reg: String,
   |         ^^^
   |
   = note: `-D dead-code` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(dead_code)]`

error: could not compile `intrinsic-test` (bin "intrinsic-test") due to 1 previous error
Error: Process completed with exit code 101.

@Amanieu
Copy link
Member

Amanieu commented Mar 31, 2024

This seems to be fallout from rust-lang/rust#119552 which was recently merged. Just add #[allow(dead_code)] on that field to ignore the lint.

@rfcbot
Copy link

rfcbot commented Apr 9, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

psst @Amanieu, I wasn't able to add the finished-final-comment-period label, please do so.

@bors
Copy link
Contributor

bors commented Apr 10, 2024

☔ The latest upstream changes (presumably 24068c7) made this pull request unmergeable. Please resolve the merge conflicts.

@Amanieu
Copy link
Member

Amanieu commented Apr 11, 2024

Can you squash and rebase?

@tarcieri tarcieri force-pushed the stabilize-aarch64-sha3-intrinsics branch from e5f23f9 to 4a5204f Compare April 11, 2024 23:32
@tarcieri
Copy link
Contributor Author

@Amanieu done

@Amanieu Amanieu merged commit 841733a into rust-lang:master Apr 12, 2024
27 checks passed
@tarcieri tarcieri deleted the stabilize-aarch64-sha3-intrinsics branch April 12, 2024 13:28
tarcieri added a commit to tarcieri/rust-stdarch that referenced this pull request Apr 12, 2024
Amanieu pushed a commit that referenced this pull request Apr 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants