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

rustc: Forbid #[inline(always)] with #[target_feature] #49425

Merged
merged 1 commit into from
Mar 30, 2018

Conversation

alexcrichton
Copy link
Member

Once a target feature is enabled for a function that means that it in general
can't be inlined into other functions which don't have that target feature
enabled. This can cause both safety and LLVM issues if we were to actually
inline it, so #[inline(always)] both can't be respected and would be an error
if we did so!

Today LLVM doesn't inline functions with different #[target_feature]
annotations, but it turns out that if one is tagged with #[inline(always)]
it'll override this and cause scary LLVM error to arise!

This commit fixes this issue by forbidding these two attributes to be used in
conjunction with one another.

Closes rust-lang/stdarch#404

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(rust_highfive has picked a reviewer for you, use r? to override)

@gnzlbg
Copy link
Contributor

gnzlbg commented Mar 27, 2018

The discussion in the RFC2045 concluded that this is the right first step. Whether #[inline(always)]+#[target_feature] makes sense or not can be resolved later. The RFC discussion suggested that maybe it should happen before stabilization, but since allowing this is backwards compatible, we can allow this whenever..

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 27, 2018

📌 Commit bd5048e has been approved by petrochenkov

@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 Mar 27, 2018
@petrochenkov
Copy link
Contributor

Wait, Travis fails.
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Mar 27, 2018
Once a target feature is enabled for a function that means that it in general
can't be inlined into other functions which don't have that target feature
enabled. This can cause both safety and LLVM issues if we were to actually
inline it, so `#[inline(always)]` both can't be respected and would be an error
if we did so!

Today LLVM doesn't inline functions with different `#[target_feature]`
annotations, but it turns out that if one is tagged with `#[inline(always)]`
it'll override this and cause scary LLVM error to arise!

This commit fixes this issue by forbidding these two attributes to be used in
conjunction with one another.

cc rust-lang/stdarch#404
@alexcrichton
Copy link
Member Author

@bors: r=petrochenkov

@bors
Copy link
Contributor

bors commented Mar 27, 2018

📌 Commit bd5048e has been approved by petrochenkov

@alexcrichton alexcrichton force-pushed the disallow-inline-always branch from bd5048e to 38d48ef Compare March 27, 2018 21:38
@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 27, 2018
@alexcrichton
Copy link
Member Author

@bors: r=petrochenkov

@bors
Copy link
Contributor

bors commented Mar 30, 2018

📌 Commit 38d48ef has been approved by petrochenkov

@bors
Copy link
Contributor

bors commented Mar 30, 2018

⌛ Testing commit 38d48ef with merge a6f1c6a...

bors added a commit that referenced this pull request Mar 30, 2018
…enkov

rustc: Forbid #[inline(always)] with #[target_feature]

Once a target feature is enabled for a function that means that it in general
can't be inlined into other functions which don't have that target feature
enabled. This can cause both safety and LLVM issues if we were to actually
inline it, so `#[inline(always)]` both can't be respected and would be an error
if we did so!

Today LLVM doesn't inline functions with different `#[target_feature]`
annotations, but it turns out that if one is tagged with `#[inline(always)]`
it'll override this and cause scary LLVM error to arise!

This commit fixes this issue by forbidding these two attributes to be used in
conjunction with one another.

Closes rust-lang/stdarch#404
@bors
Copy link
Contributor

bors commented Mar 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing a6f1c6a to master...

@bors bors merged commit 38d48ef into rust-lang:master Mar 30, 2018
@alexcrichton alexcrichton deleted the disallow-inline-always branch April 20, 2018 06:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improper usage of #[inline(always)] and #[target_feature] results in LLVM instruction selection error
5 participants