[BasicAA][LAA] Don't use same-block phis in cross iteration mode #116802
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
In 4de3184 we exposed BasicAA's cross-iteration mode for use in LAA, so we can handle selects with equal conditions correctly (where the select condition is not actually equal across iterations). However, if we replace the selects with equivalent phis, the issue still exists. In the phi case, we effectively still have an assumption that the condition(s) that control which phi arg is used will be the same across iterations. Fix this by disabling this case in cross-iteration mode.
|
@llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesIn 4de3184 we exposed BasicAA's cross-iteration mode for use in LAA, so we can handle selects with equal conditions correctly (where the select condition is not actually equal across iterations). However, if we replace the selects with equivalent phis, the issue still exists. In the phi case, we effectively still have an assumption that the condition(s) that control which phi arg is used will be the same across iterations. Fix this by disabling this phi handling in cross-iteration mode. (I'm not entirely sure whether this is also needed when BasicAA enables cross-iteration mode during internal phi recursion, but I wouldn't be surprised if that's the case.) Full diff: https://github.com/llvm/llvm-project/pull/116802.diff 2 Files Affected:
diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
index 1dcdad01f4c809..08bb11bb07ac54 100644
--- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
@@ -1447,9 +1447,10 @@ AliasResult BasicAAResult::aliasPHI(const PHINode *PN, LocationSize PNSize,
return AliasResult::NoAlias;
// If the values are PHIs in the same block, we can do a more precise
// as well as efficient check: just check for aliases between the values
- // on corresponding edges.
+ // on corresponding edges. Don't do this if we are analyzing across
+ // iterations, as we may pick a different phi entry in different iterations.
if (const PHINode *PN2 = dyn_cast<PHINode>(V2))
- if (PN2->getParent() == PN->getParent()) {
+ if (PN2->getParent() == PN->getParent() && !AAQI.MayBeCrossIteration) {
std::optional<AliasResult> Alias;
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
AliasResult ThisAlias = AAQI.AAR.alias(
diff --git a/llvm/test/Analysis/LoopAccessAnalysis/select-dependence.ll b/llvm/test/Analysis/LoopAccessAnalysis/select-dependence.ll
index 7d8a25f022e7d0..5b01efd821c474 100644
--- a/llvm/test/Analysis/LoopAccessAnalysis/select-dependence.ll
+++ b/llvm/test/Analysis/LoopAccessAnalysis/select-dependence.ll
@@ -44,8 +44,13 @@ exit:
define void @test_phi(ptr noalias %x, ptr noalias %y, ptr noalias %z) {
; CHECK-LABEL: 'test_phi'
; CHECK-NEXT: loop:
-; CHECK-NEXT: Memory dependences are safe
+; CHECK-NEXT: Report: unsafe dependent memory operations in loop. Use #pragma clang loop distribute(enable) to allow loop distribution to attempt to isolate the offending operations into a separate loop
+; CHECK-NEXT: Unsafe indirect dependence.
; CHECK-NEXT: Dependences:
+; CHECK-NEXT: IndirectUnsafe:
+; CHECK-NEXT: %load = load double, ptr %gep.sel, align 8 ->
+; CHECK-NEXT: store double %load, ptr %gep.sel2, align 8
+; CHECK-EMPTY:
; CHECK-NEXT: Run-time memory checks:
; CHECK-NEXT: Grouped accesses:
; CHECK-EMPTY:
|
|
Ping |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/73/builds/9221 Here is the relevant piece of the build log for the reference |
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.
In 4de3184 we exposed BasicAA's cross-iteration mode for use in LAA, so we can handle selects with equal conditions correctly (where the select condition is not actually equal across iterations).
However, if we replace the selects with equivalent phis, the issue still exists. In the phi case, we effectively still have an assumption that the condition(s) that control which phi arg is used will be the same across iterations. Fix this by disabling this phi handling in cross-iteration mode.
(I'm not entirely sure whether this is also needed when BasicAA enables cross-iteration mode during internal phi recursion, but I wouldn't be surprised if that's the case.)