Improve handling of lifetime intrinsics once more #27999
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
The issues that the comments referred to were fixed before the PR even landed but we never got around to remove the hack of skipping the lifetime start.
Combining them seemed like a good idea at the time, but turns out that handling lifetimes separately makes it somewhat easier to handle cases where we don't want the intrinsics, and let's you see more easily where the start/end pairs are.
… items Function arguments are live for the whole function scope, so adding lifetime intrinsics around them adds no value. The same is true for drop hint allocas and everything else that goes directly through lvalue_scratch_datum. So the easiest fix is to emit lifetime intrinsics only for lvalue datums that are created in to_lvalue_datum_in_scope(). The reduces peak memory usage and LLVM times by about 1-4%, depending on the crate.
|
r? @pnkfelix (rust_highfive has picked a reviewer for you, use r? to override) |
|
@bors r+ |
|
📌 Commit 9a15d66 has been approved by |
bors
added a commit
that referenced
this pull request
Aug 27, 2015
The major change here is in the tiny commit at the end and makes it so that we no longer emit lifetime intrinsics for allocas for function arguments. They are live for the whole function anyway, so the intrinsics add no value. This makes the resulting IR more clear, and reduces the peak memory usage and LLVM times by about 1-4%, depending on the crate. The remaining changes are just preparatory cleanups and fixes for missing lifetime intrinsics.
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.
The major change here is in the tiny commit at the end and makes it so that we no longer emit lifetime intrinsics for allocas for function arguments. They are live for the whole function anyway, so the intrinsics add no value. This makes the resulting IR more clear, and reduces the peak memory usage and LLVM times by about 1-4%, depending on the crate.
The remaining changes are just preparatory cleanups and fixes for missing lifetime intrinsics.