Skip to content

Commit 753c51b

Browse files
authored
[AST] Fix size merging for MustAlias sets (#73820)
AST checks aliasing with MustAlias sets by only checking the representative pointer (getSomePointer). This is only correct if the Size and AATags information of that pointer also includes the Size/AATags of all other pointers in the set. When we add a new pointer to the AliasSet, we do perform this update (see the code in AliasSet::addPointer). However, if a pointer already in the MustAlias set is used with a new size, we currently do not update the representative pointer, resulting in miscompilations. Fix this by adding the missing update. This is a targeted fix using the current representation. There are a couple of alternatives: * For MustAlias sets, don't store per-pointer Size/AATags at all. This would make it clear that there is only one set of common Size/AATags for all pointers. * Check against all pointers in the set even for MustAlias. This is what #65731 proposes to do as part of a larger change to AST representation. Fixes #64897.
1 parent 1087940 commit 753c51b

File tree

2 files changed

+12
-6
lines changed

2 files changed

+12
-6
lines changed

llvm/lib/Analysis/AliasSetTracker.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,16 @@ AliasSet &AliasSetTracker::getAliasSetFor(const MemoryLocation &MemLoc) {
348348
// due to a quirk of alias analysis behavior. Since alias(undef, undef)
349349
// is NoAlias, mergeAliasSetsForPointer(undef, ...) will not find the
350350
// the right set for undef, even if it exists.
351-
if (Entry.updateSizeAndAAInfo(Size, AAInfo))
351+
if (Entry.updateSizeAndAAInfo(Size, AAInfo)) {
352352
mergeAliasSetsForPointer(Pointer, Size, AAInfo, MustAliasAll);
353+
354+
// For MustAlias sets, also update Size/AAInfo of the representative
355+
// pointer.
356+
AliasSet &AS = *Entry.getAliasSet(*this);
357+
if (AS.isMustAlias())
358+
if (AliasSet::PointerRec *P = AS.getSomePointer())
359+
P->updateSizeAndAAInfo(Size, AAInfo);
360+
}
353361
// Return the set!
354362
return *Entry.getAliasSet(*this)->getForwardedTarget(*this);
355363
}

llvm/test/Transforms/LICM/pr64897.ll

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 4
22
; RUN: opt -S -passes=licm < %s | FileCheck %s
33

4-
; FIXME: This is a miscompile.
54
define void @test(i1 %c, i8 %x) {
65
; CHECK-LABEL: define void @test(
76
; CHECK-SAME: i1 [[C:%.*]], i8 [[X:%.*]]) {
@@ -10,17 +9,16 @@ define void @test(i1 %c, i8 %x) {
109
; CHECK-NEXT: [[P:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 8
1110
; CHECK-NEXT: [[P_COPY:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 8
1211
; CHECK-NEXT: [[P2:%.*]] = getelementptr inbounds i8, ptr [[A]], i64 12
13-
; CHECK-NEXT: [[P2_PROMOTED:%.*]] = load i8, ptr [[P2]], align 1
1412
; CHECK-NEXT: br label [[LOOP:%.*]]
1513
; CHECK: loop:
16-
; CHECK-NEXT: [[TMP0:%.*]] = phi i8 [ 0, [[LOOP]] ], [ [[P2_PROMOTED]], [[START:%.*]] ]
1714
; CHECK-NEXT: store i32 286331153, ptr [[P]], align 4
1815
; CHECK-NEXT: store i32 34, ptr [[P_COPY]], align 4
1916
; CHECK-NEXT: store i64 3689348814741910323, ptr [[P_COPY]], align 4
20-
; CHECK-NEXT: call void @use(i8 [[TMP0]])
17+
; CHECK-NEXT: [[VAL:%.*]] = load i8, ptr [[P2]], align 1
18+
; CHECK-NEXT: call void @use(i8 [[VAL]])
19+
; CHECK-NEXT: store i8 0, ptr [[P2]], align 1
2120
; CHECK-NEXT: br i1 [[C]], label [[LOOP]], label [[EXIT:%.*]]
2221
; CHECK: exit:
23-
; CHECK-NEXT: store i8 0, ptr [[P2]], align 1
2422
; CHECK-NEXT: ret void
2523
;
2624
start:

0 commit comments

Comments
 (0)