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.

Sign up for GitHub

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

Jump to bottom

Add more *-unwind ABI variants #93561

Merged
merged 1 commit into from
Feb 8, 2022
Merged

Add more *-unwind ABI variants #93561

merged 1 commit into from
Feb 8, 2022

Conversation

Amanieu
Copy link
Member

@Amanieu Amanieu commented Feb 1, 2022

The following *-unwind ABIs are now supported:

  • "C-unwind"
  • "cdecl-unwind"
  • "stdcall-unwind"
  • "fastcall-unwind"
  • "vectorcall-unwind"
  • "thiscall-unwind"
  • "aapcs-unwind"
  • "win64-unwind"
  • "sysv64-unwind"
  • "system-unwind"

cc @rust-lang/wg-ffi-unwind

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 1, 2022
@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive
Copy link
Collaborator

⚠️ Warning ⚠️

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 1, 2022
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@lcnr
Copy link
Contributor

lcnr commented Feb 2, 2022

r? @nagisa maybe

@rust-highfive rust-highfive assigned nagisa and unassigned lcnr Feb 2, 2022
@nagisa nagisa mentioned this pull request Feb 2, 2022
Closed
nagisa
nagisa reviewed Feb 2, 2022
View reviewed changes
compiler/rustc_ast_passes/src/feature_gate.rs
"sysv64-unwind" => {
gate_feature_post!(
&self,
c_unwind,
Copy link
Member

Choose a reason for hiding this comment

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

I wanted to say these may want their own feature gate. I could see us wanting to consider the stabilization of the C-unwind separately from most of the others.

But IIRC c_unwind feature right now not only allows user to use the -unwind ABI but also fundamentally changes the behaviour of the programs, so I guess using the same gate here as well is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

IMO the stabilization should be tied to that of C-unwind (except for ABIs that are unstable for other reasons). If we decide to describe ABIs with the "*-unwind" pattern then this should cover all ABIs where it makes sense.

src/test/codegen/unwind-abis/cdecl-unwind-abi.rs Outdated
@@ -0,0 +1,31 @@
// needs-llvm-components: x86
// compile-flags: --target=i686-pc-windows-msvc --crate-type=rlib -Cno-prepopulate-passes
Copy link
Member

Choose a reason for hiding this comment

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

Removing --target=i686-... would give more test coverage as the CC is supported for all the targets.

Copy link
Member

@nagisa nagisa Feb 2, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

r=me once this specific comment is resolved and history is squashed a bit. The rest are just observations/nits.

src/test/ui/codemap_tests/unicode.stderr
@@ -4,7 +4,7 @@ error[E0703]: invalid ABI: found `路濫狼á́́`
LL | extern "路濫狼á́́" fn foo() {}
| ^^^^^^^^^ invalid ABI
|
= help: valid ABIs: Rust, C, C-unwind, cdecl, stdcall, stdcall-unwind, fastcall, vectorcall, thiscall, thiscall-unwind, aapcs, win64, sysv64, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, wasm, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted
= help: valid ABIs: Rust, C, C-unwind, cdecl, cdecl-unwind, stdcall, stdcall-unwind, fastcall, fastcall-unwind, vectorcall, vectorcall-unwind, thiscall, thiscall-unwind, aapcs, aapcs-unwind, win64, win64-unwind, sysv64, sysv64-unwind, ptx-kernel, msp430-interrupt, x86-interrupt, amdgpu-kernel, efiapi, avr-interrupt, avr-non-blocking-interrupt, C-cmse-nonsecure-call, wasm, system, system-unwind, rust-intrinsic, rust-call, platform-intrinsic, unadjusted
Copy link
Member

Choose a reason for hiding this comment

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

I think we're at a point where we should look into doing something about this help message. I filed #93601.

@Amanieu
Add more *-unwind ABI variants
547b4e6 
The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"
@Amanieu
Copy link
Member Author

Amanieu commented Feb 2, 2022

@bors r=nagisa

@bors
Copy link
Contributor

bors commented Feb 2, 2022

📌 Commit 547b4e6 has been approved by nagisa

@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 Feb 2, 2022
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
02873fa 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc `@rust-lang/wg-ffi-unwind`
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
008870c 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc ``@rust-lang/wg-ffi-unwind``
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 3, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
00daa4a 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc ```@rust-lang/wg-ffi-unwind```
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 3, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
feb28e9 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc ````@rust-lang/wg-ffi-unwind````
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 6, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 6, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
25be507 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc `````@rust-lang/wg-ffi-unwind`````
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 6, 2022
Closed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 7, 2022
@matthiaskrgr
Rollup merge of rust-lang#93561 - Amanieu:more-unwind-abi, r=nagisa
b737bc4 
Add more *-unwind ABI variants

The following *-unwind ABIs are now supported:
- "C-unwind"
- "cdecl-unwind"
- "stdcall-unwind"
- "fastcall-unwind"
- "vectorcall-unwind"
- "thiscall-unwind"
- "aapcs-unwind"
- "win64-unwind"
- "sysv64-unwind"
- "system-unwind"

cc ``````@rust-lang/wg-ffi-unwind``````
@matthiaskrgr matthiaskrgr mentioned this pull request Feb 7, 2022
Closed
@matthiaskrgr
Copy link
Member

Might have caused #93729 (comment)
@bors rollup=iffy

@Amanieu
Copy link
Member Author

Amanieu commented Feb 7, 2022

That seems unlikely but I can't see any obvious candidates for the failure.

@nikomatsakis
Copy link
Contributor

This is great, thanks @Amanieu

@bors
Copy link
Contributor

bors commented Feb 8, 2022

⌛ Testing commit 547b4e6 with merge 2a8dbdb...

@bors
Copy link
Contributor

bors commented Feb 8, 2022

☀️ Test successful - checks-actions
Approved by: nagisa
Pushing 2a8dbdb to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 8, 2022
@bors bors merged commit 2a8dbdb into rust-lang:master Feb 8, 2022
@rustbot rustbot added this to the 1.60.0 milestone Feb 8, 2022
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (2a8dbdb): comparison url.

Summary: This benchmark run did not return any relevant results. 82 results were found to be statistically significant but too small to be relevant.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

@beetrees beetrees mentioned this pull request Apr 28, 2024
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nagisa nagisa nagisa left review comments

Assignees

@nagisa nagisa

Labels
merged-by-bors This PR was explicitly merged by bors. 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.
Projects
None yet
Milestone
1.60.0
Development

Successfully merging this pull request may close these issues.

None yet
10 participants
@Amanieu @rust-highfive @rust-log-analyzer @lcnr @bors @matthiaskrgr @nikomatsakis @rust-timer @nagisa @rustbot