Skip to content

Commit

Permalink
[Attributor] Deal with complex PHI nodes better during AAPointerInfo
Browse files Browse the repository at this point in the history 
We were quite conservative when it came to PHI node handling to avoid
recursive reasoning. Now we check more direct if we have seen a PHI
already or not. This allows non-recursive PHI chains to be handled.

This also exposed a bug as we did only model the effect of one loop
traversal. `phi_no_store_3` has been adapted to show how we would have
used `undef` instead of `1` before. With this patch we don't replace
it at all, which is expected as we do not argue about loop iterations
(or alignments).
  • Loading branch information
@jdoerfert
jdoerfert committed Jul 20, 2022
1 parent 142897d commit ad98ef8
Show file tree
Hide file tree
Showing 2 changed files with 64 additions and 36 deletions.
29 changes: 15 additions & 14 deletions llvm/lib/Transforms/IPO/AttributorAttributes.cpp
View file
Expand Up @@ -1310,6 +1310,7 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl {
if (isa<PHINode>(Usr)) {
// Note the order here, the Usr access might change the map, CurPtr is
// already in it though.
bool IsFirstPHIUser = !OffsetInfoMap.count(Usr);
OffsetInfo &UsrOI = OffsetInfoMap[Usr];
OffsetInfo &PtrOI = OffsetInfoMap[CurPtr];
// Check if the PHI is invariant (so far).
Expand All @@ -1325,25 +1326,25 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl {
}

// Check if the PHI operand is not dependent on the PHI itself.
// TODO: This is not great as we look at the pointer type. However, it
// is unclear where the Offset size comes from with typeless pointers.
APInt Offset(
DL.getIndexSizeInBits(CurPtr->getType()->getPointerAddressSpace()),
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);
Value *CurPtrBase = CurPtr->stripAndAccumulateConstantOffsets(
DL, Offset, /* AllowNonInbounds */ true);
auto It = OffsetInfoMap.find(CurPtrBase);
if (It != OffsetInfoMap.end()) {
Offset += It->getSecond().Offset;
if (IsFirstPHIUser || Offset == UsrOI.Offset)
return HandlePassthroughUser(Usr, PtrOI, Follow);
LLVM_DEBUG(dbgs()
<< "[AAPointerInfo] PHI operand pointer offset mismatch "
<< *CurPtr << " in " << *Usr << "\n");
} else {
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand is too complex "
<< *CurPtr << " in " << *Usr << "\n");
}

// 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;
Expand Down Expand Up @@ -1381,7 +1382,7 @@ struct AAPointerInfoFloating : public AAPointerInfoImpl {
Changed = translateAndAddState(A, CSArgPI,
OffsetInfoMap[CurPtr].Offset, *CB) |
Changed;
return true;
return isValidState();
}
LLVM_DEBUG(dbgs() << "[AAPointerInfo] Call user not handled " << *CB
<< "\n");
Expand Down
71 changes: 49 additions & 22 deletions llvm/test/Transforms/Attributor/value-simplify-pointer-info.ll
View file
Expand Up @@ -3068,14 +3068,14 @@ define i8 @phi_no_store_2() {
; IS__TUNIT_OPM: loop:
; IS__TUNIT_OPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a2 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
; IS__TUNIT_OPM-NEXT: [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
; IS__TUNIT_OPM-NEXT: store i8 1, i8* [[P]], align 2
; IS__TUNIT_OPM-NEXT: [[G]] = getelementptr i8, i8* bitcast (i32* @a2 to i8*), i64 2
; IS__TUNIT_OPM-NEXT: [[O]] = add nsw i8 [[I]], 1
; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; IS__TUNIT_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
; IS__TUNIT_OPM: end:
; IS__TUNIT_OPM-NEXT: [[L21:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 2), align 2
; IS__TUNIT_OPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], 0
; IS__TUNIT_OPM-NEXT: [[L22:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 3), align 1
; IS__TUNIT_OPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], [[L22]]
; IS__TUNIT_OPM-NEXT: ret i8 [[ADD]]
;
; IS__TUNIT_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn
Expand All @@ -3086,14 +3086,14 @@ define i8 @phi_no_store_2() {
; IS__TUNIT_NPM: loop:
; IS__TUNIT_NPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a2 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
; IS__TUNIT_NPM-NEXT: [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
; IS__TUNIT_NPM-NEXT: store i8 1, i8* [[P]], align 2
; IS__TUNIT_NPM-NEXT: [[G]] = getelementptr i8, i8* bitcast (i32* @a2 to i8*), i64 2
; IS__TUNIT_NPM-NEXT: [[O]] = add nsw i8 [[I]], 1
; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; IS__TUNIT_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
; IS__TUNIT_NPM: end:
; IS__TUNIT_NPM-NEXT: [[L21:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 2), align 2
; IS__TUNIT_NPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], 0
; IS__TUNIT_NPM-NEXT: [[L22:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 3), align 1
; IS__TUNIT_NPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], [[L22]]
; IS__TUNIT_NPM-NEXT: ret i8 [[ADD]]
;
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind
Expand All @@ -3104,14 +3104,14 @@ define i8 @phi_no_store_2() {
; IS__CGSCC_OPM: loop:
; IS__CGSCC_OPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a2 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
; IS__CGSCC_OPM-NEXT: [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
; IS__CGSCC_OPM-NEXT: store i8 1, i8* [[P]], align 2
; IS__CGSCC_OPM-NEXT: [[G]] = getelementptr i8, i8* bitcast (i32* @a2 to i8*), i64 2
; IS__CGSCC_OPM-NEXT: [[O]] = add nsw i8 [[I]], 1
; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; IS__CGSCC_OPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
; IS__CGSCC_OPM: end:
; IS__CGSCC_OPM-NEXT: [[L21:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 2), align 2
; IS__CGSCC_OPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], 0
; IS__CGSCC_OPM-NEXT: [[L22:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 3), align 1
; IS__CGSCC_OPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], [[L22]]
; IS__CGSCC_OPM-NEXT: ret i8 [[ADD]]
;
; IS__CGSCC_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn
Expand All @@ -3122,14 +3122,14 @@ define i8 @phi_no_store_2() {
; IS__CGSCC_NPM: loop:
; IS__CGSCC_NPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a2 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
; IS__CGSCC_NPM-NEXT: [[I:%.*]] = phi i8 [ 0, [[ENTRY]] ], [ [[O:%.*]], [[LOOP]] ]
; IS__CGSCC_NPM-NEXT: store i8 1, i8* [[P]], align 2
; IS__CGSCC_NPM-NEXT: [[G]] = getelementptr i8, i8* bitcast (i32* @a2 to i8*), i64 2
; IS__CGSCC_NPM-NEXT: [[O]] = add nsw i8 [[I]], 1
; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; IS__CGSCC_NPM-NEXT: br i1 [[C]], label [[END:%.*]], label [[LOOP]]
; IS__CGSCC_NPM: end:
; IS__CGSCC_NPM-NEXT: [[L21:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 2), align 2
; IS__CGSCC_NPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], 0
; IS__CGSCC_NPM-NEXT: [[L22:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a2 to i8*), i64 3), align 1
; IS__CGSCC_NPM-NEXT: [[ADD:%.*]] = add i8 [[L21]], [[L22]]
; IS__CGSCC_NPM-NEXT: ret i8 [[ADD]]
;
entry:
Expand All @@ -3153,10 +3153,11 @@ end:
}

define i8 @phi_no_store_3() {
; IS__TUNIT_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly
; IS__TUNIT_OPM: Function Attrs: nofree norecurse nosync nounwind
; IS__TUNIT_OPM-LABEL: define {{[^@]+}}@phi_no_store_3
; IS__TUNIT_OPM-SAME: () #[[ATTR7]] {
; IS__TUNIT_OPM-SAME: () #[[ATTR3]] {
; IS__TUNIT_OPM-NEXT: entry:
; IS__TUNIT_OPM-NEXT: store i8 0, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__TUNIT_OPM-NEXT: br label [[LOOP:%.*]]
; IS__TUNIT_OPM: loop:
; IS__TUNIT_OPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a3 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
Expand All @@ -3166,12 +3167,18 @@ define i8 @phi_no_store_3() {
; IS__TUNIT_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; 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: [[L31:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 2), align 2
; IS__TUNIT_OPM-NEXT: [[L32:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__TUNIT_OPM-NEXT: [[ADD:%.*]] = add i8 [[L31]], [[L32]]
; IS__TUNIT_OPM-NEXT: [[L34:%.*]] = load i8, i8* bitcast (i32* getelementptr inbounds (i32, i32* @a3, i64 1) to i8*), align 4
; IS__TUNIT_OPM-NEXT: [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
; IS__TUNIT_OPM-NEXT: ret i8 [[ADD2]]
;
; IS__TUNIT_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn writeonly
; IS__TUNIT_NPM: Function Attrs: nofree norecurse nosync nounwind willreturn
; IS__TUNIT_NPM-LABEL: define {{[^@]+}}@phi_no_store_3
; IS__TUNIT_NPM-SAME: () #[[ATTR5]] {
; IS__TUNIT_NPM-SAME: () #[[ATTR3]] {
; IS__TUNIT_NPM-NEXT: entry:
; IS__TUNIT_NPM-NEXT: store i8 0, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__TUNIT_NPM-NEXT: br label [[LOOP:%.*]]
; IS__TUNIT_NPM: loop:
; IS__TUNIT_NPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a3 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
Expand All @@ -3181,12 +3188,18 @@ define i8 @phi_no_store_3() {
; IS__TUNIT_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; 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: [[L31:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 2), align 2
; IS__TUNIT_NPM-NEXT: [[L32:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__TUNIT_NPM-NEXT: [[ADD:%.*]] = add i8 [[L31]], [[L32]]
; IS__TUNIT_NPM-NEXT: [[L34:%.*]] = load i8, i8* bitcast (i32* getelementptr inbounds (i32, i32* @a3, i64 1) to i8*), align 4
; IS__TUNIT_NPM-NEXT: [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
; IS__TUNIT_NPM-NEXT: ret i8 [[ADD2]]
;
; 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_3
; IS__CGSCC_OPM-SAME: () #[[ATTR10:[0-9]+]] {
; IS__CGSCC_OPM-SAME: () #[[ATTR9]] {
; IS__CGSCC_OPM-NEXT: entry:
; IS__CGSCC_OPM-NEXT: store i8 0, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__CGSCC_OPM-NEXT: br label [[LOOP:%.*]]
; IS__CGSCC_OPM: loop:
; IS__CGSCC_OPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a3 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
Expand All @@ -3196,12 +3209,18 @@ define i8 @phi_no_store_3() {
; IS__CGSCC_OPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; 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: [[L31:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 2), align 2
; IS__CGSCC_OPM-NEXT: [[L32:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__CGSCC_OPM-NEXT: [[ADD:%.*]] = add i8 [[L31]], [[L32]]
; IS__CGSCC_OPM-NEXT: [[L34:%.*]] = load i8, i8* bitcast (i32* getelementptr inbounds (i32, i32* @a3, i64 1) to i8*), align 4
; IS__CGSCC_OPM-NEXT: [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
; IS__CGSCC_OPM-NEXT: ret i8 [[ADD2]]
;
; 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_3
; IS__CGSCC_NPM-SAME: () #[[ATTR7]] {
; IS__CGSCC_NPM-SAME: () #[[ATTR5]] {
; IS__CGSCC_NPM-NEXT: entry:
; IS__CGSCC_NPM-NEXT: store i8 0, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__CGSCC_NPM-NEXT: br label [[LOOP:%.*]]
; IS__CGSCC_NPM: loop:
; IS__CGSCC_NPM-NEXT: [[P:%.*]] = phi i8* [ bitcast (i32* @a3 to i8*), [[ENTRY:%.*]] ], [ [[G:%.*]], [[LOOP]] ]
Expand All @@ -3211,7 +3230,12 @@ define i8 @phi_no_store_3() {
; IS__CGSCC_NPM-NEXT: [[C:%.*]] = icmp eq i8 [[O]], 7
; 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: [[L31:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 2), align 2
; IS__CGSCC_NPM-NEXT: [[L32:%.*]] = load i8, i8* getelementptr (i8, i8* bitcast (i32* @a3 to i8*), i64 3), align 1
; IS__CGSCC_NPM-NEXT: [[ADD:%.*]] = add i8 [[L31]], [[L32]]
; IS__CGSCC_NPM-NEXT: [[L34:%.*]] = load i8, i8* bitcast (i32* getelementptr inbounds (i32, i32* @a3, i64 1) to i8*), align 4
; IS__CGSCC_NPM-NEXT: [[ADD2:%.*]] = add i8 [[ADD]], [[L34]]
; IS__CGSCC_NPM-NEXT: ret i8 [[ADD2]]
;
entry:
%b = bitcast i32* @a3 to i8*
Expand All @@ -3232,7 +3256,10 @@ end:
%s32 = getelementptr i8, i8* %b, i64 3
%l32 = load i8, i8* %s32
%add = add i8 %l31, %l32
ret i8 %add
%s34 = getelementptr i8, i8* %b, i64 4
%l34 = load i8, i8* %s34
%add2 = add i8 %add, %l34
ret i8 %add2
}

define i8 @cast_and_load_1() {
Expand Down Expand Up @@ -3296,7 +3323,7 @@ define void @recursive_load_store(i64 %N, i32 %v) {
;
; IS__CGSCC_OPM: Function Attrs: nofree norecurse nosync nounwind writeonly
; IS__CGSCC_OPM-LABEL: define {{[^@]+}}@recursive_load_store
; IS__CGSCC_OPM-SAME: (i64 [[N:%.*]], i32 [[V:%.*]]) #[[ATTR10]] {
; IS__CGSCC_OPM-SAME: (i64 [[N:%.*]], i32 [[V:%.*]]) #[[ATTR10:[0-9]+]] {
; IS__CGSCC_OPM-NEXT: entry:
; IS__CGSCC_OPM-NEXT: br label [[FOR_COND:%.*]]
; IS__CGSCC_OPM: for.cond:
Expand Down

0 comments on commit ad98ef8

Please sign in to comment.