-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Declare by-value on-stack parameters to be noalias #12296
Merged
Merged
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
Function parameters that are to be passed by value but don't fit into a single register are currently passed by creating a copy on the stack and passing a pointer to that copy to the callee. Since the copy is made just for the function call, there are no aliases. For example, this sometimes allows LLVM to eliminate unnecessary calls to drop glue. Given ````rust struct Foo { a: int, b: Option<~str>, } extern { fn eat(eat: Option<~str>); } pub fn foo(v: Foo) { match v { Foo { a: _, b } => unsafe { eat(b) } } } ```` LLVM currently can't eliminate the drop call for the string, because it only sees a _pointer_ to Foo, for which it has to expect an alias. So we get: ````llvm ; Function Attrs: uwtable define void @_ZN3foo20h9f32c90ae7201edbxaa4v0.0E(%struct.Foo* nocapture) unnamed_addr #0 { "_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit": %1 = getelementptr inbounds %struct.Foo* %0, i64 0, i32 1, i32 0 %2 = load { i64, i64, [0 x i8] }** %1, align 8 store { i64, i64, [0 x i8] }* null, { i64, i64, [0 x i8] }** %1, align 8 %3 = ptrtoint { i64, i64, [0 x i8] }* %2 to i64 %.fca.0.insert = insertvalue { i64 } undef, i64 %3, 0 tail call void @eat({ i64 } %.fca.0.insert) %4 = load { i64, i64, [0 x i8] }** %1, align 8 %5 = icmp eq { i64, i64, [0 x i8] }* %4, null br i1 %5, label %_ZN3Foo9glue_drop17hf611996539d3036fE.exit, label %"_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i" "_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i": ; preds = %"_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit" %6 = bitcast { i64, i64, [0 x i8] }* %4 to i8* tail call void @free(i8* %6) #1 br label %_ZN3Foo9glue_drop17hf611996539d3036fE.exit _ZN3Foo9glue_drop17hf611996539d3036fE.exit: ; preds = %"_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit", %"_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i" ret void } ```` But with the `noalias` attribute, it can safely optimize that to: ````llvm define void @_ZN3foo20hd28431f929f0d6c4xaa4v0.0E(%struct.Foo* noalias nocapture) unnamed_addr #0 { _ZN3Foo9glue_drop17he9afbc09d4e9c851E.exit: %1 = getelementptr inbounds %struct.Foo* %0, i64 0, i32 1, i32 0 %2 = load { i64, i64, [0 x i8] }** %1, align 8 store { i64, i64, [0 x i8] }* null, { i64, i64, [0 x i8] }** %1, align 8 %3 = ptrtoint { i64, i64, [0 x i8] }* %2 to i64 %.fca.0.insert = insertvalue { i64 } undef, i64 %3, 0 tail call void @eat({ i64 } %.fca.0.insert) ret void } ````
bors
added a commit
that referenced
this pull request
Feb 15, 2014
Function parameters that are to be passed by value but don't fit into a single register are currently passed by creating a copy on the stack and passing a pointer to that copy to the callee. Since the copy is made just for the function call, there are no aliases. For example, this sometimes allows LLVM to eliminate unnecessary calls to drop glue. Given ````rust struct Foo { a: int, b: Option<~str>, } extern { fn eat(eat: Option<~str>); } pub fn foo(v: Foo) { match v { Foo { a: _, b } => unsafe { eat(b) } } } ```` LLVM currently can't eliminate the drop call for the string, because it only sees a _pointer_ to Foo, for which it has to expect an alias. So we get: ````llvm ; Function Attrs: uwtable define void @_ZN3foo20h9f32c90ae7201edbxaa4v0.0E(%struct.Foo* nocapture) unnamed_addr #0 { "_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit": %1 = getelementptr inbounds %struct.Foo* %0, i64 0, i32 1, i32 0 %2 = load { i64, i64, [0 x i8] }** %1, align 8 store { i64, i64, [0 x i8] }* null, { i64, i64, [0 x i8] }** %1, align 8 %3 = ptrtoint { i64, i64, [0 x i8] }* %2 to i64 %.fca.0.insert = insertvalue { i64 } undef, i64 %3, 0 tail call void @eat({ i64 } %.fca.0.insert) %4 = load { i64, i64, [0 x i8] }** %1, align 8 %5 = icmp eq { i64, i64, [0 x i8] }* %4, null br i1 %5, label %_ZN3Foo9glue_drop17hf611996539d3036fE.exit, label %"_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i" "_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i": ; preds = %"_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit" %6 = bitcast { i64, i64, [0 x i8] }* %4 to i8* tail call void @free(i8* %6) #1 br label %_ZN3Foo9glue_drop17hf611996539d3036fE.exit _ZN3Foo9glue_drop17hf611996539d3036fE.exit: ; preds = %"_ZN34std..option..Option$LT$$UP$str$GT$9glue_drop17hc39b3015f3b9c69dE.exit", %"_ZN8_$UP$str9glue_drop17h15dbdbe2b8897a98E.exit.i.i" ret void } ```` But with the `noalias` attribute, it can safely optimize that to: ````llvm define void @_ZN3foo20hd28431f929f0d6c4xaa4v0.0E(%struct.Foo* noalias nocapture) unnamed_addr #0 { _ZN3Foo9glue_drop17he9afbc09d4e9c851E.exit: %1 = getelementptr inbounds %struct.Foo* %0, i64 0, i32 1, i32 0 %2 = load { i64, i64, [0 x i8] }** %1, align 8 store { i64, i64, [0 x i8] }* null, { i64, i64, [0 x i8] }** %1, align 8 %3 = ptrtoint { i64, i64, [0 x i8] }* %2 to i64 %.fca.0.insert = insertvalue { i64 } undef, i64 %3, 0 tail call void @eat({ i64 } %.fca.0.insert) ret void } ````
I think we need to have more codegen tests for this kind of changes. Getting to 1.0 and introducing backwards compatibility also means having a good way to avoid introducing regressions for this kind of optimization. I'd like to see one for this fix too. |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jul 25, 2022
… r=jonas-schievink feat: add a "Add attribute" assist This generalizes the "Add `#[derive]`" assist and supports adding `#[must_use]` and `#[inline]`. Removes `#[must_use]` from the "Generate getter/setter" assist, addressing the last point in rust-lang/rust-analyzer#12273. Closes rust-lang/rust-analyzer#12273.
flip1995
pushed a commit
to flip1995/rust
that referenced
this pull request
Feb 26, 2024
fix: documentation of `blocks_in_conditions` lint Updated documentation + example of `blocks_in_conditions` lint, which has been updated recently to include `match` statements as well. changelog: none
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.
Function parameters that are to be passed by value but don't fit into a
single register are currently passed by creating a copy on the stack and
passing a pointer to that copy to the callee. Since the copy is made
just for the function call, there are no aliases.
For example, this sometimes allows LLVM to eliminate unnecessary calls
to drop glue. Given
LLVM currently can't eliminate the drop call for the string, because it
only sees a pointer to Foo, for which it has to expect an alias. So we
get:
But with the
noalias
attribute, it can safely optimize that to: