Skip to content

Commit ab096b7

Browse files
jdoerferttstellar
jdoerfert
authored and
tstellar
committed
[Attributor][FIX] Handle recurrences (PHIs) in AAPointerInfo explicitly
PHI nodes are not pass through but change their value, we have to account for that to avoid missing stores. Follow up for D107798 to fix PR51249 for good. Differential Revision: https://reviews.llvm.org/D107808 (cherry picked from commit e7e3585)
1 parent 3490cba commit ab096b7

File tree

2 files changed

+86
-22
lines changed

2 files changed

+86
-22
lines changed

‎llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,12 +1156,16 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl {
11561156
ChangeStatus Changed = ChangeStatus::UNCHANGED;
11571157
Value &AssociatedValue = getAssociatedValue();
11581158
struct OffsetInfo {
1159-
int64_t Offset = 0;
1159+
int64_t Offset = OffsetAndSize::Unknown;
1160+
1161+
bool operator==(const OffsetInfo &OI) const {
1162+
return Offset == OI.Offset;
1163+
}
11601164
};
11611165

11621166
const DataLayout &DL = A.getDataLayout();
11631167
DenseMap<Value *, OffsetInfo> OffsetInfoMap;
1164-
OffsetInfoMap[&AssociatedValue] = {};
1168+
OffsetInfoMap[&AssociatedValue] = OffsetInfo{0};
11651169

11661170
auto HandlePassthroughUser = [&](Value *Usr, OffsetInfo &PtrOI,
11671171
bool &Follow) {
@@ -1219,8 +1223,48 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl {
12191223
Follow = true;
12201224
return true;
12211225
}
1222-
if (isa<CastInst>(Usr) || isa<PHINode>(Usr) || isa<SelectInst>(Usr))
1226+
if (isa<CastInst>(Usr) || isa<SelectInst>(Usr))
12231227
return HandlePassthroughUser(Usr, PtrOI, Follow);
1228+
1229+
// For PHIs we need to take care of the recurrence explicitly as the value
1230+
// might change while we iterate through a loop. For now, we give up if
1231+
// the PHI is not invariant.
1232+
if (isa<PHINode>(Usr)) {
1233+
// Check if the PHI is invariant (so far).
1234+
OffsetInfo &UsrOI = OffsetInfoMap[Usr];
1235+
if (UsrOI == PtrOI)
1236+
return true;
1237+
1238+
// Check if the PHI operand has already an unknown offset as we can't
1239+
// improve on that anymore.
1240+
if (PtrOI.Offset == OffsetAndSize::Unknown) {
1241+
UsrOI = PtrOI;
1242+
Follow = true;
1243+
return true;
1244+
}
1245+
1246+
// Check if the PHI operand is not dependent on the PHI itself.
1247+
APInt Offset(DL.getIndexTypeSizeInBits(AssociatedValue.getType()), 0);
1248+
if (&AssociatedValue == CurPtr->stripAndAccumulateConstantOffsets(
1249+
DL, Offset, /* AllowNonInbounds */ true)) {
1250+
if (Offset != PtrOI.Offset) {
1251+
LLVM_DEBUG(dbgs()
1252+
<< "[AAPointerInfo] PHI operand pointer offset mismatch "
1253+
<< *CurPtr << " in " << *Usr << "\n");
1254+
return false;
1255+
}
1256+
return HandlePassthroughUser(Usr, PtrOI, Follow);
1257+
}
1258+
1259+
// TODO: Approximate in case we know the direction of the recurrence.
1260+
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand is too complex "
1261+
<< *CurPtr << " in " << *Usr << "\n");
1262+
UsrOI = PtrOI;
1263+
UsrOI.Offset = OffsetAndSize::Unknown;
1264+
Follow = true;
1265+
return true;
1266+
}
1267+
12241268
if (auto *LoadI = dyn_cast<LoadInst>(Usr))
12251269
return handleAccess(A, *LoadI, *CurPtr, /* Content */ nullptr,
12261270
AccessKind::AK_READ, PtrOI.Offset, Changed,

‎llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll

Lines changed: 39 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2890,7 +2890,9 @@ define i8 @phi_store() {
28902890
; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2
28912891
; IS__TUNIT_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
28922892
; IS__TUNIT_OPM: end:
2893-
; IS__TUNIT_OPM-NEXT: ret i8 1
2893+
; IS__TUNIT_OPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1
2894+
; IS__TUNIT_OPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1
2895+
; IS__TUNIT_OPM-NEXT: ret i8 [[L]]
28942896
;
28952897
; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind readnone willreturn
28962898
; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@phi_store
@@ -2907,7 +2909,9 @@ define i8 @phi_store() {
29072909
; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2
29082910
; IS__TUNIT_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
29092911
; IS__TUNIT_NPM: end:
2910-
; IS__TUNIT_NPM-NEXT: ret i8 1
2912+
; IS__TUNIT_NPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1
2913+
; IS__TUNIT_NPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1
2914+
; IS__TUNIT_NPM-NEXT: ret i8 [[L]]
29112915
;
29122916
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind readnone
29132917
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_store
@@ -2924,7 +2928,9 @@ define i8 @phi_store() {
29242928
; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2
29252929
; IS__CGSCC_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
29262930
; IS__CGSCC_OPM: end:
2927-
; IS__CGSCC_OPM-NEXT: ret i8 1
2931+
; IS__CGSCC_OPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1
2932+
; IS__CGSCC_OPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1
2933+
; IS__CGSCC_OPM-NEXT: ret i8 [[L]]
29282934
;
29292935
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn
29302936
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@phi_store
@@ -2941,7 +2947,9 @@ define i8 @phi_store() {
29412947
; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2
29422948
; IS__CGSCC_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
29432949
; IS__CGSCC_NPM: end:
2944-
; IS__CGSCC_NPM-NEXT: ret i8 1
2950+
; IS__CGSCC_NPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1
2951+
; IS__CGSCC_NPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1
2952+
; IS__CGSCC_NPM-NEXT: ret i8 [[L]]
29452953
;
29462954
entry:
29472955
%a = alloca i16
@@ -2963,9 +2971,9 @@ end:
29632971

29642972
; FIXME: This function returns 1.
29652973
define i8 @phi_no_store_1() {
2966-
; IS__TUNIT_OPM: Function Attrs: nofree nosync nounwind writeonly
2974+
; IS__TUNIT_OPM: Function Attrs: nofree nosync nounwind
29672975
; IS__TUNIT_OPM-LABEL: define {{[^@]+}}@phi_no_store_1
2968-
; IS__TUNIT_OPM-SAME: () #[[ATTR6]] {
2976+
; IS__TUNIT_OPM-SAME: () #[[ATTR2]] {
29692977
; IS__TUNIT_OPM-NEXT: entry:
29702978
; IS__TUNIT_OPM-NEXT: br label [[LOOP:%.*]]
29712979
; IS__TUNIT_OPM: loop:
@@ -2976,11 +2984,14 @@ define i8 @phi_no_store_1() {
29762984
; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3
29772985
; IS__TUNIT_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
29782986
; IS__TUNIT_OPM: end:
2979-
; IS__TUNIT_OPM-NEXT: ret i8 0
2987+
; IS__TUNIT_OPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2
2988+
; IS__TUNIT_OPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1
2989+
; IS__TUNIT_OPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]]
2990+
; IS__TUNIT_OPM-NEXT: ret i8 [[ADD]]
29802991
;
2981-
; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind willreturn writeonly
2992+
; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind willreturn
29822993
; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@phi_no_store_1
2983-
; IS__TUNIT_NPM-SAME: () #[[ATTR4]] {
2994+
; IS__TUNIT_NPM-SAME: () #[[ATTR2]] {
29842995
; IS__TUNIT_NPM-NEXT: entry:
29852996
; IS__TUNIT_NPM-NEXT: br label [[LOOP:%.*]]
29862997
; IS__TUNIT_NPM: loop:
@@ -2991,9 +3002,12 @@ define i8 @phi_no_store_1() {
29913002
; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3
29923003
; IS__TUNIT_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
29933004
; IS__TUNIT_NPM: end:
2994-
; IS__TUNIT_NPM-NEXT: ret i8 0
3005+
; IS__TUNIT_NPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2
3006+
; IS__TUNIT_NPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1
3007+
; IS__TUNIT_NPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]]
3008+
; IS__TUNIT_NPM-NEXT: ret i8 [[ADD]]
29953009
;
2996-
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly
3010+
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind
29973011
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_1
29983012
; IS__CGSCC_OPM-SAME: () #[[ATTR9:[0-9]+]] {
29993013
; IS__CGSCC_OPM-NEXT: entry:
@@ -3006,11 +3020,14 @@ define i8 @phi_no_store_1() {
30063020
; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3
30073021
; IS__CGSCC_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
30083022
; IS__CGSCC_OPM: end:
3009-
; IS__CGSCC_OPM-NEXT: ret i8 0
3023+
; IS__CGSCC_OPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2
3024+
; IS__CGSCC_OPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1
3025+
; IS__CGSCC_OPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]]
3026+
; IS__CGSCC_OPM-NEXT: ret i8 [[ADD]]
30103027
;
3011-
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn writeonly
3028+
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn
30123029
; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@phi_no_store_1
3013-
; IS__CGSCC_NPM-SAME: () #[[ATTR4]] {
3030+
; IS__CGSCC_NPM-SAME: () #[[ATTR5]] {
30143031
; IS__CGSCC_NPM-NEXT: entry:
30153032
; IS__CGSCC_NPM-NEXT: br label [[LOOP:%.*]]
30163033
; IS__CGSCC_NPM: loop:
@@ -3021,7 +3038,10 @@ define i8 @phi_no_store_1() {
30213038
; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3
30223039
; IS__CGSCC_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
30233040
; IS__CGSCC_NPM: end:
3024-
; IS__CGSCC_NPM-NEXT: ret i8 0
3041+
; IS__CGSCC_NPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2
3042+
; IS__CGSCC_NPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1
3043+
; IS__CGSCC_NPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]]
3044+
; IS__CGSCC_NPM-NEXT: ret i8 [[ADD]]
30253045
;
30263046
entry:
30273047
%b = bitcast i32* @a1 to i8*
@@ -3080,7 +3100,7 @@ define i8 @phi_no_store_2() {
30803100
;
30813101
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind
30823102
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_2
3083-
; IS__CGSCC_OPM-SAME: () #[[ATTR10:[0-9]+]] {
3103+
; IS__CGSCC_OPM-SAME: () #[[ATTR9]] {
30843104
; IS__CGSCC_OPM-NEXT: entry:
30853105
; IS__CGSCC_OPM-NEXT: br label [[LOOP:%.*]]
30863106
; IS__CGSCC_OPM: loop:
@@ -3162,7 +3182,7 @@ define i8 @phi_no_store_3() {
31623182
;
31633183
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly
31643184
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_3
3165-
; IS__CGSCC_OPM-SAME: () #[[ATTR9]] {
3185+
; IS__CGSCC_OPM-SAME: () #[[ATTR10:[0-9]+]] {
31663186
; IS__CGSCC_OPM-NEXT: entry:
31673187
; IS__CGSCC_OPM-NEXT: br label [[LOOP:%.*]]
31683188
; IS__CGSCC_OPM: loop:
@@ -3274,8 +3294,8 @@ end:
32743294
; IS__CGSCC_OPM: attributes #[[ATTR6]] = { argmemonly nofree norecurse nosync nounwind willreturn }
32753295
; IS__CGSCC_OPM: attributes #[[ATTR7]] = { nofree norecurse nosync nounwind readonly willreturn }
32763296
; IS__CGSCC_OPM: attributes #[[ATTR8]] = { nofree norecurse nosync nounwind readnone }
3277-
; IS__CGSCC_OPM: attributes #[[ATTR9]] = { nofree norecurse nosync nounwind writeonly }
3278-
; IS__CGSCC_OPM: attributes #[[ATTR10]] = { nofree norecurse nosync nounwind }
3297+
; IS__CGSCC_OPM: attributes #[[ATTR9]] = { nofree norecurse nosync nounwind }
3298+
; IS__CGSCC_OPM: attributes #[[ATTR10]] = { nofree norecurse nosync nounwind writeonly }
32793299
; IS__CGSCC_OPM: attributes #[[ATTR11]] = { willreturn }
32803300
; IS__CGSCC_OPM: attributes #[[ATTR12]] = { nounwind willreturn writeonly }
32813301
; IS__CGSCC_OPM: attributes #[[ATTR13]] = { nounwind writeonly }

0 commit comments

Comments
 (0)