-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Allow specifying alignment for functions #81234
Conversation
r? @lcnr (rust-highfive has picked a reviewer for you, use r? to override) |
|
Not sure how that submodule got added, but removed 🤔 |
This comment has been minimized.
This comment has been minimized.
nominating for |
This seems reasonable to me, to add on an unstable basis. (There needs to be a feature-gate added.) I also think many codebases will want to set this for all functions using a compiler option (e.g. set in a Cargo profile). But supporting it on individual functions seems reasonable as well. EDIT: We discussed this in today's lang-team meeting, and this was the general consensus:
|
Awesome! I should hopefully have some time soon to do that :) |
Team member @joshtriplett has proposed to merge this. The next step is review by the rest of the tagged team members: Concerns:
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. |
383465c
to
dd575a9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I think I added all the requests, any other changes that need made? |
@rfcbot resolve feature-gate Thank you! |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
☔ The latest upstream changes (presumably #82443) made this pull request unmergeable. Please resolve the merge conflicts. |
Shouldn't this be going through the RFC process? |
if ci passes, please squash your commits after that r=me @bors delegate+ |
✌️ @repnop can now approve this pull request |
@bors r=lcnr |
📌 Commit 448d076 has been approved by |
☀️ Test successful - checks-actions |
What are the language-level effects of this? Is this mainly a hint for how to put the compiled binary code together, or is his somehow observable inside Rust (e.g. by adding new kinds of UB)? |
yes, its not intended to introduce any visible effects from within Rust, but to allow telling LLVM that the function needs to be aligned to a specific boundary for situations in which that's necessary and you can't rely on the native alignment placing it at an appropriate address (e.g. so it can be inserted into a hardware register, in my case)
I think the only thing that may have been glossed over in this PR as it stands right now is being able to under-align functions, which I'll need to investigate to see if we need to add some additional checks to the |
…ulacrum [beta] Bootstrap from stable This is the follow up to master/beta promotion, as well as the first round of backports: * Revert "Allow specifying alignment for functions rust-lang#81234" * Revert rust-lang#85176 addition of clone_from for ManuallyDrop rust-lang#85758 * rustdoc: revert deref recur to resume inclusion of impl ExtTrait<Local> for ExtType rust-lang#84867 * [beta] Update cargo rust-lang#86563 r? `@Mark-Simulacrum`
Fixes #75072
This allows the user to specify alignment for functions, which can be useful for low level work where functions need to necessarily be aligned to a specific value.
I believe the error cases not covered in the match are caught earlier based on my testing so I had them just return
None
.