From d2cc9397470e2502c92b6115094b497f8c1d823e Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Tue, 10 Aug 2021 04:06:59 -0500 Subject: [PATCH] [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 e7e3585cde0b08152a8cbf54029794d07c15963d) --- lib/Transforms/IPO/AttributorAttributes.cpp | 50 +++++++++++++++- .../Attributor/value-simplify-pointer-info.ll | 58 +++++++++++++------ 2 files changed, 86 insertions(+), 22 deletions(-) diff --git a/lib/Transforms/IPO/AttributorAttributes.cpp b/lib/Transforms/IPO/AttributorAttributes.cpp index 98ce286d513..e8661967210 100644 --- a/lib/Transforms/IPO/AttributorAttributes.cpp +++ b/lib/Transforms/IPO/AttributorAttributes.cpp @@ -1156,12 +1156,16 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl { ChangeStatus Changed = ChangeStatus::UNCHANGED; Value &AssociatedValue = getAssociatedValue(); struct OffsetInfo { - int64_t Offset = 0; + int64_t Offset = OffsetAndSize::Unknown; + + bool operator==(const OffsetInfo &OI) const { + return Offset == OI.Offset; + } }; const DataLayout &DL = A.getDataLayout(); DenseMap OffsetInfoMap; - OffsetInfoMap[&AssociatedValue] = {}; + OffsetInfoMap[&AssociatedValue] = OffsetInfo{0}; auto HandlePassthroughUser = [&](Value *Usr, OffsetInfo &PtrOI, bool &Follow) { @@ -1219,8 +1223,48 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl { Follow = true; return true; } - if (isa(Usr) || isa(Usr) || isa(Usr)) + if (isa(Usr) || isa(Usr)) return HandlePassthroughUser(Usr, PtrOI, Follow); + + // For PHIs we need to take care of the recurrence explicitly as the value + // might change while we iterate through a loop. For now, we give up if + // the PHI is not invariant. + if (isa(Usr)) { + // Check if the PHI is invariant (so far). + OffsetInfo &UsrOI = OffsetInfoMap[Usr]; + if (UsrOI == PtrOI) + return true; + + // Check if the PHI operand has already an unknown offset as we can't + // improve on that anymore. + if (PtrOI.Offset == OffsetAndSize::Unknown) { + UsrOI = PtrOI; + Follow = true; + return true; + } + + // Check if the PHI operand is not dependent on the PHI itself. + APInt Offset(DL.getIndexTypeSizeInBits(AssociatedValue.getType()), 0); + if (&AssociatedValue == CurPtr->stripAndAccumulateConstantOffsets( + DL, Offset, /* AllowNonInbounds */ true)) { + if (Offset != PtrOI.Offset) { + LLVM_DEBUG(dbgs() + << "[AAPointerInfo] PHI operand pointer offset mismatch " + << *CurPtr << " in " << *Usr << "\n"); + return false; + } + return HandlePassthroughUser(Usr, PtrOI, Follow); + } + + // TODO: Approximate in case we know the direction of the recurrence. + LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand is too complex " + << *CurPtr << " in " << *Usr << "\n"); + UsrOI = PtrOI; + UsrOI.Offset = OffsetAndSize::Unknown; + Follow = true; + return true; + } + if (auto *LoadI = dyn_cast(Usr)) return handleAccess(A, *LoadI, *CurPtr, /* Content */ nullptr, AccessKind::AK_READ, PtrOI.Offset, Changed, diff --git a/test/Transforms/Attributor/value-simplify-pointer-info.ll b/test/Transforms/Attributor/value-simplify-pointer-info.ll index 1a97e2b1c31..fdb974f003a 100644 --- a/test/Transforms/Attributor/value-simplify-pointer-info.ll +++ b/test/Transforms/Attributor/value-simplify-pointer-info.ll @@ -2890,7 +2890,9 @@ define i8 @phi_store() { ; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2 ; IS__TUNIT_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__TUNIT_OPM: end: -; IS__TUNIT_OPM-NEXT: ret i8 1 +; IS__TUNIT_OPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1 +; IS__TUNIT_OPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1 +; IS__TUNIT_OPM-NEXT: ret i8 [[L]] ; ; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind readnone willreturn ; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@phi_store @@ -2907,7 +2909,9 @@ define i8 @phi_store() { ; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2 ; IS__TUNIT_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__TUNIT_NPM: end: -; IS__TUNIT_NPM-NEXT: ret i8 1 +; IS__TUNIT_NPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1 +; IS__TUNIT_NPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1 +; IS__TUNIT_NPM-NEXT: ret i8 [[L]] ; ; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind readnone ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_store @@ -2924,7 +2928,9 @@ define i8 @phi_store() { ; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2 ; IS__CGSCC_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__CGSCC_OPM: end: -; IS__CGSCC_OPM-NEXT: ret i8 1 +; IS__CGSCC_OPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1 +; IS__CGSCC_OPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1 +; IS__CGSCC_OPM-NEXT: ret i8 [[L]] ; ; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind readnone willreturn ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@phi_store @@ -2941,7 +2947,9 @@ define i8 @phi_store() { ; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 2 ; IS__CGSCC_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__CGSCC_NPM: end: -; IS__CGSCC_NPM-NEXT: ret i8 1 +; IS__CGSCC_NPM-NEXT: [[S:%.*]] = getelementptr i8, i8* [[B]], i64 1 +; IS__CGSCC_NPM-NEXT: [[L:%.*]] = load i8, i8* [[S]], align 1 +; IS__CGSCC_NPM-NEXT: ret i8 [[L]] ; entry: %a = alloca i16 @@ -2963,9 +2971,9 @@ end: ; FIXME: This function returns 1. define i8 @phi_no_store_1() { -; IS__TUNIT_OPM: Function Attrs: nofree nosync nounwind writeonly +; IS__TUNIT_OPM: Function Attrs: nofree nosync nounwind ; IS__TUNIT_OPM-LABEL: define {{[^@]+}}@phi_no_store_1 -; IS__TUNIT_OPM-SAME: () #[[ATTR6]] { +; IS__TUNIT_OPM-SAME: () #[[ATTR2]] { ; IS__TUNIT_OPM-NEXT: entry: ; IS__TUNIT_OPM-NEXT: br label [[LOOP:%.*]] ; IS__TUNIT_OPM: loop: @@ -2976,11 +2984,14 @@ define i8 @phi_no_store_1() { ; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3 ; IS__TUNIT_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__TUNIT_OPM: end: -; IS__TUNIT_OPM-NEXT: ret i8 0 +; IS__TUNIT_OPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2 +; IS__TUNIT_OPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1 +; IS__TUNIT_OPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]] +; IS__TUNIT_OPM-NEXT: ret i8 [[ADD]] ; -; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind willreturn writeonly +; IS__TUNIT_NPM: Function Attrs: nofree nosync nounwind willreturn ; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@phi_no_store_1 -; IS__TUNIT_NPM-SAME: () #[[ATTR4]] { +; IS__TUNIT_NPM-SAME: () #[[ATTR2]] { ; IS__TUNIT_NPM-NEXT: entry: ; IS__TUNIT_NPM-NEXT: br label [[LOOP:%.*]] ; IS__TUNIT_NPM: loop: @@ -2991,9 +3002,12 @@ define i8 @phi_no_store_1() { ; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3 ; IS__TUNIT_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__TUNIT_NPM: end: -; IS__TUNIT_NPM-NEXT: ret i8 0 +; IS__TUNIT_NPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2 +; IS__TUNIT_NPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1 +; IS__TUNIT_NPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]] +; IS__TUNIT_NPM-NEXT: ret i8 [[ADD]] ; -; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly +; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_1 ; IS__CGSCC_OPM-SAME: () #[[ATTR9:[0-9]+]] { ; IS__CGSCC_OPM-NEXT: entry: @@ -3006,11 +3020,14 @@ define i8 @phi_no_store_1() { ; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3 ; IS__CGSCC_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__CGSCC_OPM: end: -; IS__CGSCC_OPM-NEXT: ret i8 0 +; IS__CGSCC_OPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2 +; IS__CGSCC_OPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1 +; IS__CGSCC_OPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]] +; IS__CGSCC_OPM-NEXT: ret i8 [[ADD]] ; -; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn writeonly +; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn ; IS__CGSCC_NPM-LABEL: define {{[^@]+}}@phi_no_store_1 -; IS__CGSCC_NPM-SAME: () #[[ATTR4]] { +; IS__CGSCC_NPM-SAME: () #[[ATTR5]] { ; IS__CGSCC_NPM-NEXT: entry: ; IS__CGSCC_NPM-NEXT: br label [[LOOP:%.*]] ; IS__CGSCC_NPM: loop: @@ -3021,7 +3038,10 @@ define i8 @phi_no_store_1() { ; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 3 ; IS__CGSCC_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]] ; IS__CGSCC_NPM: end: -; IS__CGSCC_NPM-NEXT: ret i8 0 +; IS__CGSCC_NPM-NEXT: [[L11:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 2), align 2 +; IS__CGSCC_NPM-NEXT: [[L12:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a1 to i8*), i64 3), align 1 +; IS__CGSCC_NPM-NEXT: [[ADD:%.*]] = add i8 [[L11]], [[L12]] +; IS__CGSCC_NPM-NEXT: ret i8 [[ADD]] ; entry: %b = bitcast i32* @a1 to i8* @@ -3080,7 +3100,7 @@ define i8 @phi_no_store_2() { ; ; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_2 -; IS__CGSCC_OPM-SAME: () #[[ATTR10:[0-9]+]] { +; IS__CGSCC_OPM-SAME: () #[[ATTR9]] { ; IS__CGSCC_OPM-NEXT: entry: ; IS__CGSCC_OPM-NEXT: br label [[LOOP:%.*]] ; IS__CGSCC_OPM: loop: @@ -3162,7 +3182,7 @@ define i8 @phi_no_store_3() { ; ; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly ; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@phi_no_store_3 -; IS__CGSCC_OPM-SAME: () #[[ATTR9]] { +; IS__CGSCC_OPM-SAME: () #[[ATTR10:[0-9]+]] { ; IS__CGSCC_OPM-NEXT: entry: ; IS__CGSCC_OPM-NEXT: br label [[LOOP:%.*]] ; IS__CGSCC_OPM: loop: @@ -3274,8 +3294,8 @@ end: ; IS__CGSCC_OPM: attributes #[[ATTR6]] = { argmemonly nofree norecurse nosync nounwind willreturn } ; IS__CGSCC_OPM: attributes #[[ATTR7]] = { nofree norecurse nosync nounwind readonly willreturn } ; IS__CGSCC_OPM: attributes #[[ATTR8]] = { nofree norecurse nosync nounwind readnone } -; IS__CGSCC_OPM: attributes #[[ATTR9]] = { nofree norecurse nosync nounwind writeonly } -; IS__CGSCC_OPM: attributes #[[ATTR10]] = { nofree norecurse nosync nounwind } +; IS__CGSCC_OPM: attributes #[[ATTR9]] = { nofree norecurse nosync nounwind } +; IS__CGSCC_OPM: attributes #[[ATTR10]] = { nofree norecurse nosync nounwind writeonly } ; IS__CGSCC_OPM: attributes #[[ATTR11]] = { willreturn } ; IS__CGSCC_OPM: attributes #[[ATTR12]] = { nounwind willreturn writeonly } ; IS__CGSCC_OPM: attributes #[[ATTR13]] = { nounwind writeonly }