Add Swift function call ABI #64582
Closed
Add Swift function call ABI #64582
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @zackmdavis (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
nvzqz
force-pushed
the
swiftcc
branch
2 times, most recently
from
17e29f5
to
0b86f54
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
cc @rust-lang/lang I would be OK with landing this in an experimental fashion but this is definitely RFC territory. I think we need to seriously consider whether making support for the Swift ABI something Rust the language should support (i.e. this would require that cranelift also supports it if that is to become a supported backend). |
|
I'm inclined to think this needs an RFC, mainly in that we need to decide if we really want to commit to having support for this. I wouldn't want to add this even experimentally without a commitment that we intend to ultimately support this. I think this is more in the domain of T-compiler than T-lang though. |
Centril
added
needs-rfc
Fair. Added labels accordingly.
If we add this it will be a commitment that all Rust compilers/backends should support the Swift ABI (and possibly with changes to the Swift ABI) and it would end up in the reference with the relevant text. It's not something that I would be OK with on a "only this compiler supports it"-basis. So this is primarily a language team matter. However, I am also very happy for the compiler team to also review any RFC as the added technical expertise and judgement re. |
|
The difficulty for other rustc backends like cranelift is clearly an issue on which the compiler team has much more expertise |
|
Well, for cranelift specifically, I would consult the cranelift devs; I don't believe any of those are on the compiler team. But this doesn't have to be one team vs. the other -- both perspectives are useful. |
|
I'll definitely write up an RFC for this and move the community discussion to there. I was planning to do so anyway, but the work in me writing this code in the short term felt like less work for me than writing prose. Regarding other Rust compilers, I wasn't sure what the stance was regarding non-"official" compiler implementations (namely based on cranelift). I imagine that cross-language compatibility is a non-goal for Rust and so it's often deferred as a community effort. However, I think it's a space worth exploring and so far the community has done that with projects like wasm-bindgen and the now-deprecated curryrs. Rust is in a unique position in its ability to expose multiple calling conventions. In this case with Swift, LLVM has already done most of the legwork for Rust and so it's rather trivial to support this in @Centril regarding concerns with Swift's ABI changing, its ABI is stable as of 5.0. This includes the function call convention. Swift's standard library now ships with Apple's operating systems and is dynamically linked to rather than each program packaging their own copy themselves. |
|
@nvzqz I was just passing by and saw this and wanted to add that Swift ABI is only declared stable on Darwin systems. This means that Linux and Windows ABI for Swift are not "stable". |
Personally, I'd very much like to see cranelift become an officially supported backend for Rust. We'll also need to consider what impact this has on Miri.
True, but I suspect it would be non-trivial for Cranelift, Miri, and other compilers. It would probably also be a lot of complexity to add to a language specification.
What happens in Swift 6.0 and what would it mean for us? |
|
I see no reason why we have to tie "support for the Swift ABI" to "guaranteed support for the Swift ABI on all targets and compiler backends". This should be like I don't think it's obligatory for the cranelift backend to support every possible calling convention that the LLVM backend does, even on the same target. I would propose that we accept support for this in nightly, discuss questions of how to support it in various backends in parallel, and gate stabilization on the resolution of such discussions. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I disagree with the comparison to
It seems to me important that the cranelift backend should have rough parity with the LLVM backend if it is actually going to be used, e.g. by default in debug mode in the future, and not have a multitude of portability concerns. |
It seems it is just an extension of the SysV abi with two extra registers having a special purpose. It shouldn't be too hard too implement in Cranelift. cc @sunfishcode (I think you know more about the details of Cranelift abi legalization and abi's in general than me.) |
|
Does the Swift abi needs its own classify_arg in https://github.com/rust-lang/rust/tree/master/src/librustc_target/abi/call? |
joelpalmer
added
S-waiting-on-author
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Maps to LLVM's Swift calling convention, which is listed as 16.
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
I'll get around to writing an RFC this week. Anyone have an idea as to why this keeps coming up at - found type `unsafe extern "rust-intrinsic" fn(_) -> _ {std::intrinsics::transmute::<_, _>}`
+ found type `unsafe extern "rust-intrinsic" fn(_) -> _ {std::mem::transmute::<_, _>}`
- error[E0606]: casting `unsafe extern "rust-intrinsic" fn(_) -> _ {std::intrinsics::transmute::<_, _>}` as `unsafe extern "rust-intrinsic" fn(isize) -> usize` is invalid
+ error[E0606]: casting `unsafe extern "rust-intrinsic" fn(_) -> _ {std::mem::transmute::<_, _>}` as `unsafe extern "rust-intrinsic" fn(isize) -> usize` is invalid |
|
☔ The latest upstream changes (presumably #64906) made this pull request unmergeable. Please resolve the merge conflicts. |
|
ping from triage @nvzqz, any updates on this? thanks! |
nvzqz
commented
Oct 17, 2019
| @@ -519,6 +519,9 @@ declare_features! ( | |||
| /// Allows the use of or-patterns (e.g., `0 | 1`). | |||
| (active, or_patterns, "1.38.0", Some(54883), None), | |||
|
|
|||
| /// Allows using the `Swift` ABI. | |||
| (active, abi_swift, "1.38.0", Some(0), None), | |||
There was a problem hiding this comment.
Unsure as to what Rust version I would put here?
|
@hdhoang thanks for the reminder! I'll actually write the RFC for this soon. 😄 |
Dylan-DPC-zz
added
S-blocked
|
this is blocked because it requires an rfc |
|
r? @zackmdavis |
|
Closing -- we can reopen when/if the RFC lands. |
Mark-Simulacrum
added
S-blocked-closed
and removed
S-blocked
|
Curious if an RFC was ever submitted, and/or if the plan included adding the swift_context and swift_error parameter attributes. |
|
Hey @nvzqz, just a follow up, did you get around to writing the RFC? |
|
@UebelAndre no this has been punted to my backlog since I've had to prioritize other things. I can't guarantee I'll get around to writing the RFC soon, but I do plan to get around to this within a few months. |
|
@nvzqz Sounds good, thanks for the heads up and I look forward to the RFC 😄 |
jyn514
added
S-blocked
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This teaches Rust the Swift calling convention known by LLVM (see
llvm::CallingConv).Some background: I'm working on swift-bindgen which will require calling Swift and being called by Swift without having to go through C.