[MemDep] Treat partial-overlap lifetime.start as a clobber (#211167) - #13544
Open
usama54321 wants to merge 2 commits into
Open
[MemDep] Treat partial-overlap lifetime.start as a clobber (#211167)#13544usama54321 wants to merge 2 commits into
usama54321 wants to merge 2 commits into
Conversation
When getSimplePointerDependencyFrom scans backward and reaches a @llvm.lifetime.start, it only treated the marker as a barrier when the query location MustAliased the lifetime's argument, and otherwise fell through to `continue`. For a load through a GEP into part of an alloca the alias result is PartialAlias, so the scan walked past lifetime.start as if it weren't there, and GVN Load PRE would hoist the load above it. Inspect the full alias result instead of just isMustAlias: MustAlias still returns Def and NoAlias still continues, but any partial/may overlap now returns a Clobber so lifetime.start acts as a barrier and the access cannot be moved above it. Fixes llvm#194940
Author
|
@swift-ci test llvm |
Author
|
@hnrklssn Just giving you headsup that I am cherrypicking the change on 21.x. The second commit fixes a ubsan test where a check was getting optimized away before |
Author
|
@swift-ci test llvm |
Follow-up to 5f88e49 (llvm#211167), which stops GVN from folding a load of a partially-overlapping alloca access to undef. In test15 that undef fed the local-bounds check, which then got deleted. Since all indices for the fam are out of bounds, clang emits the local-bounds trap at -O0, but -O2 was silently dropping it. The regenerated checks now keep __ubsan_handle_local_out_of_bounds_abort on that path. The rest of the diff is attribute group renumbering.
usama54321
force-pushed
the
eng/u_hameed/gvn-pre-load-hoist-cherrypick
branch
from
July 30, 2026 21:26
7d38464 to
6186477
Compare
Author
|
@swift-ci test llvm |
Author
|
@swift-ci test |
Member
|
Wouldn't this cherry-pick need to include the changes from #13528? |
Author
|
Hmm the test did not fail on the initial run. I just took a look and seems like a bunch of bounds safety work is missing/cherrypick of your change created a merge conflict. I will wait for the results for now |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
When getSimplePointerDependencyFrom scans backward and reaches a @llvm.lifetime.start, it only treated the marker as a barrier when the query location MustAliased the lifetime's argument, and otherwise fell through to
continue. For a load through a GEP into part of an alloca the alias result is PartialAlias, so the scan walked past lifetime.start as if it weren't there, and GVN Load PRE would hoist the load above it.Inspect the full alias result instead of just isMustAlias: MustAlias still returns Def and NoAlias still continues, but any partial/may overlap now returns a Clobber so lifetime.start acts as a barrier and the access cannot be moved above it.
Fixes llvm#194940