Skip to content

Commit c7633dd

Browse files
vidsinghalVidush Singhal
andauthored
[Attributor]: Ensure cycle info is not null when handling PHI in AAPointerInfo (#97321)
Ensure cycle info object is not null for simple PHI case for the test: `llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll` Debug info Before the change: ``` Accesses by bin after update: [8-12] : 1 - 9 - store i32 %0, ptr %field2, align 4 - c: %0 = load i32, ptr %val, align 4 [32-36] : 1 - 9 - store i32 %1, ptr %field8, align 4 - c: %1 = load i32, ptr %val2, align 4 [2147483647-4294967294] : 1 - 6 - %ret = load i32, ptr %x, align 4 - c: <unknown> ``` Debug info After the change: ``` Accesses by bin after update: [8-12] : 2 - 9 - store i32 %0, ptr %field2, align 4 - c: %0 = load i32, ptr %val, align 4 - 6 - %ret = load i32, ptr %x, align 4 - c: <unknown> [32-36] : 2 - 9 - store i32 %1, ptr %field8, align 4 - c: %1 = load i32, ptr %val2, align 4 - 6 - %ret = load i32, ptr %x, align 4 - c: <unknown> ``` Co-authored-by: Vidush Singhal <[email protected]>
1 parent b76100e commit c7633dd

File tree

2 files changed

+53
-13
lines changed

2 files changed

+53
-13
lines changed

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1621,13 +1621,6 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
16211621
return true;
16221622
};
16231623

1624-
const auto *F = getAnchorScope();
1625-
const auto *CI =
1626-
F ? A.getInfoCache().getAnalysisResultForFunction<CycleAnalysis>(*F)
1627-
: nullptr;
1628-
const auto *TLI =
1629-
F ? A.getInfoCache().getTargetLibraryInfoForFunction(*F) : nullptr;
1630-
16311624
auto UsePred = [&](const Use &U, bool &Follow) -> bool {
16321625
Value *CurPtr = U.get();
16331626
User *Usr = U.getUser();
@@ -1671,18 +1664,18 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
16711664
// For PHIs we need to take care of the recurrence explicitly as the value
16721665
// might change while we iterate through a loop. For now, we give up if
16731666
// the PHI is not invariant.
1674-
if (isa<PHINode>(Usr)) {
1667+
if (auto *PHI = dyn_cast<PHINode>(Usr)) {
16751668
// Note the order here, the Usr access might change the map, CurPtr is
16761669
// already in it though.
1677-
bool IsFirstPHIUser = !OffsetInfoMap.count(Usr);
1678-
auto &UsrOI = OffsetInfoMap[Usr];
1670+
bool IsFirstPHIUser = !OffsetInfoMap.count(PHI);
1671+
auto &UsrOI = OffsetInfoMap[PHI];
16791672
auto &PtrOI = OffsetInfoMap[CurPtr];
16801673

16811674
// Check if the PHI operand has already an unknown offset as we can't
16821675
// improve on that anymore.
16831676
if (PtrOI.isUnknown()) {
16841677
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand offset unknown "
1685-
<< *CurPtr << " in " << *Usr << "\n");
1678+
<< *CurPtr << " in " << *PHI << "\n");
16861679
Follow = !UsrOI.isUnknown();
16871680
UsrOI.setUnknown();
16881681
return true;
@@ -1705,7 +1698,8 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
17051698
auto It = OffsetInfoMap.find(CurPtrBase);
17061699
if (It == OffsetInfoMap.end()) {
17071700
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand is too complex "
1708-
<< *CurPtr << " in " << *Usr << "\n");
1701+
<< *CurPtr << " in " << *PHI
1702+
<< " (base: " << *CurPtrBase << ")\n");
17091703
UsrOI.setUnknown();
17101704
Follow = true;
17111705
return true;
@@ -1718,6 +1712,9 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
17181712
// Cycles reported by CycleInfo. It is sufficient to check the PHIs in
17191713
// every Cycle header; if such a node is marked unknown, this will
17201714
// eventually propagate through the whole net of PHIs in the recurrence.
1715+
const auto *CI =
1716+
A.getInfoCache().getAnalysisResultForFunction<CycleAnalysis>(
1717+
*PHI->getFunction());
17211718
if (mayBeInCycle(CI, cast<Instruction>(Usr), /* HeaderOnly */ true)) {
17221719
auto BaseOI = It->getSecond();
17231720
BaseOI.addToAll(Offset.getZExtValue());
@@ -1729,7 +1726,7 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
17291726

17301727
LLVM_DEBUG(
17311728
dbgs() << "[AAPointerInfo] PHI operand pointer offset mismatch "
1732-
<< *CurPtr << " in " << *Usr << "\n");
1729+
<< *CurPtr << " in " << *PHI << "\n");
17331730
UsrOI.setUnknown();
17341731
Follow = true;
17351732
return true;
@@ -1882,6 +1879,8 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
18821879
if (auto *CB = dyn_cast<CallBase>(Usr)) {
18831880
if (CB->isLifetimeStartOrEnd())
18841881
return true;
1882+
const auto *TLI =
1883+
A.getInfoCache().getTargetLibraryInfoForFunction(*CB->getFunction());
18851884
if (getFreedOperand(CB, TLI) == U)
18861885
return true;
18871886
if (CB->isArgOperand(&U)) {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes --check-globals --version 2
2+
; RUN: opt -aa-pipeline=basic-aa -passes=attributor -debug-only=attributor -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s 2>&1 | FileCheck %s
3+
; RUN: opt -aa-pipeline=basic-aa -passes=attributor-cgscc -debug-only=attributor -attributor-manifest-internal -attributor-annotate-decl-cs -S < %s 2>&1 | FileCheck %s
4+
; REQUIRES: asserts
5+
6+
7+
@globalBytes = internal global [1024 x i8] zeroinitializer
8+
9+
; CHECK: Accesses by bin after update:
10+
; CHECK: [8-12] : 2
11+
; CHECK: - 9 - store i32 %0, ptr %field2, align 4
12+
; CHECK: - c: %0 = load i32, ptr %val, align 4
13+
; CHECK: - 6 - %ret = load i32, ptr %x, align 4
14+
; CHECK: - c: <unknown>
15+
; CHECK: [32-36] : 2
16+
; CHECK: - 9 - store i32 %1, ptr %field8, align 4
17+
; CHECK: - c: %1 = load i32, ptr %val2, align 4
18+
; CHECK: - 6 - %ret = load i32, ptr %x, align 4
19+
; CHECK: - c: <unknown>
20+
define dso_local i32 @phi_different_offsets(ptr nocapture %val, ptr nocapture %val2, i1 %cmp) {
21+
entry:
22+
br i1 %cmp, label %then, label %else
23+
24+
then:
25+
%field2 = getelementptr i32, ptr @globalBytes, i32 2
26+
%1 = load i32, ptr %val
27+
store i32 %1, ptr %field2
28+
br label %end
29+
30+
else:
31+
%field8 = getelementptr i32, ptr @globalBytes, i32 8
32+
%2 = load i32, ptr %val2
33+
store i32 %2, ptr %field8
34+
br label %end
35+
36+
end:
37+
%x = phi ptr [ %field2, %then ], [ %field8, %else ]
38+
%ret = load i32, ptr %x
39+
ret i32 %ret
40+
41+
}

0 commit comments

Comments
 (0)