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 support for Arm64EC to the Standard Library #123144

Merged
merged 1 commit into from
Apr 18, 2024

Conversation

dpaoliello
Copy link
Contributor

@dpaoliello dpaoliello commented Mar 27, 2024

Adds the final pieces so that the standard library can be built for arm64ec-pc-windows-msvc (initially added in #119199)

  • Bumps windows-sys to 0.56.0, which adds support for Arm64EC.
  • Correctly set the isEC parameter for LLVM's writeArchive function.
  • Add #![feature(asm_experimental_arch)] to library crates where Arm64EC inline assembly is used, as it is currently unstable.

@rustbot
Copy link
Collaborator

rustbot commented Mar 27, 2024

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added O-windows Operating system: Windows S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 27, 2024
@workingjubilee
Copy link
Member

workingjubilee commented Mar 27, 2024

...I'm confused why such a pervasive library change assigned someone from docs? Rustbot? You okay over there?

@dpaoliello
Copy link
Contributor Author

...I'm confused why such a pervasive library change assigned someone from docs? Rustbot? You okay over there?

I did change some docs, so 🤷

@GuillaumeGomez
Copy link
Member

Apart from my nits, docs changes look good. 😉

However, please assign someone else for the final approval.

@dpaoliello
Copy link
Contributor Author

r? libs

@rustbot rustbot assigned m-ou-se and unassigned GuillaumeGomez Mar 27, 2024
@dpaoliello dpaoliello force-pushed the arm64eclib branch 2 times, most recently from fdb14ac to f01c81c Compare March 27, 2024 22:09
@ChrisDenton
Copy link
Member

windows_sys.rs is a generated file so any manual changes will be overwritten. See https://github.com/rust-lang/rust/blob/master/library/std/src/sys/pal/windows/c/README.md

You can override definitions by putting them in library/std/src/sys/pal/windows/c.rs but ideally fixes should be done in windows-bindgen (well, more likely the win32metadata project from which bindings are derived).

@dpaoliello
Copy link
Contributor Author

windows_sys.rs is a generated file so any manual changes will be overwritten. See https://github.com/rust-lang/rust/blob/master/library/std/src/sys/pal/windows/c/README.md

You can override definitions by putting them in library/std/src/sys/pal/windows/c.rs but ideally fixes should be done in windows-bindgen (well, more likely the win32metadata project from which bindings are derived).

Submitted a PR to add Arm64EC support in windows-rs: microsoft/windows-rs#2957

@dpaoliello
Copy link
Contributor Author

dpaoliello commented Mar 29, 2024

@rustbot blocked

Need to update windows-rs

@rustbot rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 29, 2024
@rustbot
Copy link
Collaborator

rustbot commented Apr 12, 2024

These commits modify the Cargo.lock file. Unintentional changes to Cargo.lock can be introduced when switching branches and rebasing PRs.

If this was unintentional then you should revert the changes before this PR is merged.
Otherwise, you can ignore this comment.

Some changes occurred in src/doc/rustc/src/platform-support

cc @Nilstrieb

@dpaoliello
Copy link
Contributor Author

windows-sys 0.56 has support for Arm64EC, so this PR is ready to go
@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Apr 15, 2024
Copy link
Member

@ChrisDenton ChrisDenton left a comment

Choose a reason for hiding this comment

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

The libs changes look good to me, though I've not tested it.

@dpaoliello
Copy link
Contributor Author

The libs changes look good to me, though I've not tested it.

Thanks!

Sounds like the only piece left to review is the compiler changes.
r? compiler

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 16, 2024
@rustbot rustbot assigned wesleywiser and unassigned m-ou-se Apr 16, 2024
Copy link
Member

@wesleywiser wesleywiser left a comment

Choose a reason for hiding this comment

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

Compiler changes look good to me

@wesleywiser
Copy link
Member

@bors r=GuillaumeGomez,ChrisDenton,wesleywiser

@bors
Copy link
Contributor

bors commented Apr 17, 2024

📌 Commit 32f5ca4 has been approved by GuillaumeGomez,ChrisDenton,wesleywiser

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 17, 2024
@bors
Copy link
Contributor

bors commented Apr 18, 2024

⌛ Testing commit 32f5ca4 with merge c5de414...

@bors
Copy link
Contributor

bors commented Apr 18, 2024

☀️ Test successful - checks-actions
Approved by: GuillaumeGomez,ChrisDenton,wesleywiser
Pushing c5de414 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 18, 2024
@bors bors merged commit c5de414 into rust-lang:master Apr 18, 2024
12 checks passed
@rustbot rustbot added this to the 1.79.0 milestone Apr 18, 2024
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (c5de414): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.1% [-2.1%, -2.1%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.1% [-2.1%, -2.1%] 1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 676.875s -> 677.567s (0.10%)
Artifact size: 315.38 MiB -> 315.40 MiB (0.00%)

@dpaoliello dpaoliello deleted the arm64eclib branch April 18, 2024 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. O-windows Operating system: Windows S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants