Skip to content

Commit bba4a1d

Browse files
authored
[ArgPromotion] Remove incorrect TranspBlocks set for loads. (#84835)
The TranspBlocks set was used to cache aliasing decision for all processed loads in the parent loop. This is incorrect, because each load can access a different location, which means one load not being modified in a block doesn't translate to another load not being modified in the same block. All loads access the same underlying object, so we could perhaps use a location without size for all loads and retain the cache, but that would mean we loose precision. For now, just drop the cache. Fixes #84807 PR: #84835
1 parent a3b5250 commit bba4a1d

File tree

2 files changed

+8
-12
lines changed

2 files changed

+8
-12
lines changed

llvm/lib/Transforms/IPO/ArgumentPromotion.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -653,10 +653,6 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
653653
// check to see if the pointer is guaranteed to not be modified from entry of
654654
// the function to each of the load instructions.
655655

656-
// Because there could be several/many load instructions, remember which
657-
// blocks we know to be transparent to the load.
658-
df_iterator_default_set<BasicBlock *, 16> TranspBlocks;
659-
660656
for (LoadInst *Load : Loads) {
661657
// Check to see if the load is invalidated from the start of the block to
662658
// the load itself.
@@ -670,7 +666,7 @@ static bool findArgParts(Argument *Arg, const DataLayout &DL, AAResults &AAR,
670666
// To do this, we perform a depth first search on the inverse CFG from the
671667
// loading block.
672668
for (BasicBlock *P : predecessors(BB)) {
673-
for (BasicBlock *TranspBB : inverse_depth_first_ext(P, TranspBlocks))
669+
for (BasicBlock *TranspBB : inverse_depth_first(P))
674670
if (AAR.canBasicBlockModify(*TranspBB, Loc))
675671
return false;
676672
}

llvm/test/Transforms/ArgumentPromotion/aliasing-and-non-aliasing-loads-with-clobber.ll

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,14 @@ target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:
77

88
; Test case for https://github.com/llvm/llvm-project/issues/84807.
99

10-
; FIXME: Currently the loads from @callee are moved to @caller, even though
11-
; the store in %then may aliases to load from %q.
10+
; Make sure the loads from @callee are not moved to @caller, as the store
11+
; in %then may aliases to load from %q.
1212

1313
define i32 @caller1(i1 %c) {
1414
; CHECK-LABEL: define i32 @caller1(
1515
; CHECK-SAME: i1 [[C:%.*]]) {
1616
; CHECK-NEXT: entry:
17-
; CHECK-NEXT: [[F_VAL:%.*]] = load i16, ptr @f, align 8
18-
; CHECK-NEXT: [[TMP0:%.*]] = getelementptr i8, ptr @f, i64 8
19-
; CHECK-NEXT: [[F_VAL1:%.*]] = load i64, ptr [[TMP0]], align 8
20-
; CHECK-NEXT: call void @callee1(i16 [[F_VAL]], i64 [[F_VAL1]], i1 [[C]])
17+
; CHECK-NEXT: call void @callee1(ptr noundef nonnull @f, i1 [[C]])
2118
; CHECK-NEXT: ret i32 0
2219
;
2320
entry:
@@ -27,13 +24,16 @@ entry:
2724

2825
define internal void @callee1(ptr nocapture noundef readonly %q, i1 %c) {
2926
; CHECK-LABEL: define internal void @callee1(
30-
; CHECK-SAME: i16 [[Q_0_VAL:%.*]], i64 [[Q_8_VAL:%.*]], i1 [[C:%.*]]) {
27+
; CHECK-SAME: ptr nocapture noundef readonly [[Q:%.*]], i1 [[C:%.*]]) {
3128
; CHECK-NEXT: entry:
3229
; CHECK-NEXT: br i1 [[C]], label [[THEN:%.*]], label [[EXIT:%.*]]
3330
; CHECK: then:
3431
; CHECK-NEXT: store i16 123, ptr @f, align 8
3532
; CHECK-NEXT: br label [[EXIT]]
3633
; CHECK: exit:
34+
; CHECK-NEXT: [[Q_0_VAL:%.*]] = load i16, ptr [[Q]], align 8
35+
; CHECK-NEXT: [[GEP_8:%.*]] = getelementptr inbounds i8, ptr [[Q]], i64 8
36+
; CHECK-NEXT: [[Q_8_VAL:%.*]] = load i64, ptr [[GEP_8]], align 8
3737
; CHECK-NEXT: call void @use(i16 [[Q_0_VAL]], i64 [[Q_8_VAL]])
3838
; CHECK-NEXT: ret void
3939
;

0 commit comments

Comments
 (0)