[llvm] [Attributor]: Fix Potential bug when handling PHI in AAPointerInfo (PR #97321)
Vidush Singhal via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 1 13:46:47 PDT 2024
https://github.com/vidsinghal updated https://github.com/llvm/llvm-project/pull/97321
>From b067c69f500b75d62314c5837f9ac3866abebff5 Mon Sep 17 00:00:00 2001
From: Vidush Singhal <singhal2 at ruby964.llnl.gov>
Date: Mon, 1 Jul 2024 10:06:43 -0700
Subject: [PATCH] Fix Potential bug when handling PHI in AAPointerInfo
---
.../Transforms/IPO/AttributorAttributes.cpp | 25 ++++++-----
.../Attributor/phi_bug_pointer_info.ll | 41 +++++++++++++++++++
2 files changed, 53 insertions(+), 13 deletions(-)
create mode 100644 llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll
diff --git a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
index c4b9375a53a27..25d9ab2b3598b 100644
--- a/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ b/llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -1621,13 +1621,6 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
return true;
};
- const auto *F = getAnchorScope();
- const auto *CI =
- F ? A.getInfoCache().getAnalysisResultForFunction<CycleAnalysis>(*F)
- : nullptr;
- const auto *TLI =
- F ? A.getInfoCache().getTargetLibraryInfoForFunction(*F) : nullptr;
-
auto UsePred = [&](const Use &U, bool &Follow) -> bool {
Value *CurPtr = U.get();
User *Usr = U.getUser();
@@ -1671,18 +1664,18 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
// 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<PHINode>(Usr)) {
+ if (auto *PHI = dyn_cast<PHINode>(Usr)) {
// Note the order here, the Usr access might change the map, CurPtr is
// already in it though.
- bool IsFirstPHIUser = !OffsetInfoMap.count(Usr);
- auto &UsrOI = OffsetInfoMap[Usr];
+ bool IsFirstPHIUser = !OffsetInfoMap.count(PHI);
+ auto &UsrOI = OffsetInfoMap[PHI];
auto &PtrOI = OffsetInfoMap[CurPtr];
// Check if the PHI operand has already an unknown offset as we can't
// improve on that anymore.
if (PtrOI.isUnknown()) {
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand offset unknown "
- << *CurPtr << " in " << *Usr << "\n");
+ << *CurPtr << " in " << *PHI << "\n");
Follow = !UsrOI.isUnknown();
UsrOI.setUnknown();
return true;
@@ -1705,7 +1698,8 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
auto It = OffsetInfoMap.find(CurPtrBase);
if (It == OffsetInfoMap.end()) {
LLVM_DEBUG(dbgs() << "[AAPointerInfo] PHI operand is too complex "
- << *CurPtr << " in " << *Usr << "\n");
+ << *CurPtr << " in " << *PHI
+ << " (base: " << *CurPtrBase << ")\n");
UsrOI.setUnknown();
Follow = true;
return true;
@@ -1718,6 +1712,9 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
// Cycles reported by CycleInfo. It is sufficient to check the PHIs in
// every Cycle header; if such a node is marked unknown, this will
// eventually propagate through the whole net of PHIs in the recurrence.
+ const auto *CI =
+ A.getInfoCache().getAnalysisResultForFunction<CycleAnalysis>(
+ *PHI->getFunction());
if (mayBeInCycle(CI, cast<Instruction>(Usr), /* HeaderOnly */ true)) {
auto BaseOI = It->getSecond();
BaseOI.addToAll(Offset.getZExtValue());
@@ -1729,7 +1726,7 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
LLVM_DEBUG(
dbgs() << "[AAPointerInfo] PHI operand pointer offset mismatch "
- << *CurPtr << " in " << *Usr << "\n");
+ << *CurPtr << " in " << *PHI << "\n");
UsrOI.setUnknown();
Follow = true;
return true;
@@ -1882,6 +1879,8 @@ ChangeStatus AAPointerInfoFloating::updateImpl(Attributor &A) {
if (auto *CB = dyn_cast<CallBase>(Usr)) {
if (CB->isLifetimeStartOrEnd())
return true;
+ const auto *TLI =
+ A.getInfoCache().getTargetLibraryInfoForFunction(*CB->getFunction());
if (getFreedOperand(CB, TLI) == U)
return true;
if (CB->isArgOperand(&U)) {
diff --git a/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll b/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll
new file mode 100644
index 0000000000000..bb423e10f2c72
--- /dev/null
+++ b/llvm/test/Transforms/Attributor/phi_bug_pointer_info.ll
@@ -0,0 +1,41 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --check-attributes --check-globals --version 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
+; 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
+; REQUIRES: asserts
+
+
+ at globalBytes = internal global [1024 x i8] zeroinitializer
+
+; CHECK: Accesses by bin after update:
+; CHECK: [8-12] : 2
+; CHECK: - 9 - store i32 %0, ptr %field2, align 4
+; CHECK: - c: %0 = load i32, ptr %val, align 4
+; CHECK: - 6 - %ret = load i32, ptr %x, align 4
+; CHECK: - c: <unknown>
+; CHECK: [32-36] : 2
+; CHECK: - 9 - store i32 %1, ptr %field8, align 4
+; CHECK: - c: %1 = load i32, ptr %val2, align 4
+; CHECK: - 6 - %ret = load i32, ptr %x, align 4
+; CHECK: - c: <unknown>
+define dso_local i32 @phi_different_offsets(ptr nocapture %val, ptr nocapture %val2, i1 %cmp) {
+entry:
+ br i1 %cmp, label %then, label %else
+
+then:
+ %field2 = getelementptr i32, ptr @globalBytes, i32 2
+ %1 = load i32, ptr %val
+ store i32 %1, ptr %field2
+ br label %end
+
+else:
+ %field8 = getelementptr i32, ptr @globalBytes, i32 8
+ %2 = load i32, ptr %val2
+ store i32 %2, ptr %field8
+ br label %end
+
+end:
+ %x = phi ptr [ %field2, %then ], [ %field8, %else ]
+ %ret = load i32, ptr %x
+ ret i32 %ret
+
+}
More information about the llvm-commits
mailing list