[IR] Remove size argument from lifetime intrinsics #150248
Merged
Conversation
This file contains hidden or 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
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
nikic
force-pushed
the
lifetime-size
branch
2 times, most recently
from
a56aec7 to
ef647f8
Compare
llvmbot
added
clang
nikic
added a commit
to nikic/llvm-project
that referenced
this pull request
Aug 7, 2025
Split out from llvm#150248: Use the size of the alloca instead of the size passed to the lifetime intrinsic. As a bonus, this handles dynamic allocas correctly (see the added test) instead of doing a memset with size -1...
|
I think this is now ready for review |
nikic
added a commit
that referenced
this pull request
Aug 7, 2025
Split out from #150248: Use the size of the alloca instead of the size passed to the lifetime intrinsic. As a bonus, this handles dynamic allocas correctly (see the added test) instead of doing a memset with size -1...
llvm-sync bot
pushed a commit
to arm/arm-toolchain
that referenced
this pull request
Aug 7, 2025
…154) Split out from llvm/llvm-project#150248: Use the size of the alloca instead of the size passed to the lifetime intrinsic. As a bonus, this handles dynamic allocas correctly (see the added test) instead of doing a memset with size -1...
efriedma-quic
approved these changes
Aug 7, 2025
Now that llvm#149310 has restricted lifetime intrinsics to only work on allocas, we can also drop the explicit size argument. Instead, the size is implied by the alloca. This removes the ability to only mark a prefix of an alloca alive/dead. We never used that capability, so we should remove the need to handle that possibility everywhere (many key places, including stack coloring, did not actually respect this).
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/25074 Here is the relevant piece of the build log for the reference |
maurer
added a commit
to maurer/rust
that referenced
this pull request
Aug 8, 2025
In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
maurer
added a commit
to maurer/rust
that referenced
this pull request
Aug 8, 2025
In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
maurer
added a commit
to maurer/rust
that referenced
this pull request
Aug 8, 2025
In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
maurer
added a commit
to maurer/rust
that referenced
this pull request
Aug 8, 2025
In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Aug 11, 2025
LLVM changed the way lifetime intrinsics work in llvm/llvm-project#150248. rust-lang/rust#145120 claims to fix these tests against ToT LLVM, but let's just suppress these until that goes in. Bug: 437926231 Change-Id: I4737655cdd2e4b3f00844a0b89f83c4ec11cf023 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6838098 Commit-Queue: Arthur Eubanks <[email protected]> Auto-Submit: Arthur Eubanks <[email protected]> Reviewed-by: Devon Loehr <[email protected]> Commit-Queue: Devon Loehr <[email protected]> Cr-Commit-Position: refs/heads/main@{#1499682}
krishna2803
pushed a commit
to krishna2803/llvm-project
that referenced
this pull request
Aug 12, 2025
Split out from llvm#150248: Since llvm#150944 the size passed to lifetime.start/end is considered meaningless. The lifetime always applies to the whole alloca. Accordingly remove handling for size mismatch in the StackLifetime analysis.
krishna2803
pushed a commit
to krishna2803/llvm-project
that referenced
this pull request
Aug 12, 2025
Split out from llvm#150248: Since llvm#150944 the size passed to lifetime.start/end is considered meaningless. The lifetime always applies to the whole alloca. This adjusts MemoryLocation to determine the MemoryLocation size from the alloca size, instead of using the argument.
krishna2803
pushed a commit
to krishna2803/llvm-project
that referenced
this pull request
Aug 12, 2025
Split out from llvm#150248: Since llvm#150944 the size passed to lifetime.start/end is considered meaningless. The lifetime always applies to the whole alloca. Accordingly, remove checks of the lifetime size from MemCpyOpt.
Zalathar
added a commit
to Zalathar/rust
that referenced
this pull request
Aug 13, 2025
llvm: Accept new LLVM lifetime format In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
jdonszelmann
added a commit
to jdonszelmann/rust
that referenced
this pull request
Aug 13, 2025
llvm: Accept new LLVM lifetime format In llvm/llvm-project#150248 LLVM removed the size parameter from the lifetime format. Tolerate not having that size parameter.
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.
Now that #149310 has restricted lifetime intrinsics to only work on allocas, we can also drop the explicit size argument. Instead, the size is implied by the alloca.
This removes the ability to only mark a prefix of an alloca alive/dead. We never used that capability, so we should remove the need to handle that possibility everywhere (though many key places, including stack coloring, did not actually respect this).