[llvm] [GVN] Look through select/phi when determining underlying object (PR #99509)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 22 02:08:45 PDT 2024
https://github.com/nikic updated https://github.com/llvm/llvm-project/pull/99509
>From f98349d1cf84f04fd7f72bd1753d8080a8c02318 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 15 Jul 2024 17:11:44 +0200
Subject: [PATCH 1/4] [GVN] Look through select/phi when determining underlying
object
This addresses an optimization regression in Rust we have observed
after https://github.com/llvm/llvm-project/pull/82458. We now only
perform pointer replacement if they have the same underlying object.
However, getUnderlyingObject() by default only looks through
linear chains, not selects/phis. In particular, this means that
we miss cases involving involving pointer induction variables.
This patch fixes this by introducing a new helper
getUnderlyingObjectThroughPhisAndSelects() (let me know if you
have a better name...) which basically does what getUnderlyingObjects()
does, just specialized to the case where we must arrive at a single
underlying object in the end, and with a limit on the number of
inspected values.
Doing this more expensive underlying object check has no measurable
compile-time impact on CTMark.
---
llvm/include/llvm/Analysis/ValueTracking.h | 5 +++
llvm/lib/Analysis/Loads.cpp | 5 ++-
llvm/lib/Analysis/ValueTracking.cpp | 42 ++++++++++++++++++++++
llvm/test/Transforms/GVN/condprop.ll | 6 ++--
4 files changed, 52 insertions(+), 6 deletions(-)
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index 2c2f965a3cd6f..c689fd4851f12 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -736,6 +736,11 @@ inline Value *getUnderlyingObject(Value *V, unsigned MaxLookup = 6) {
return const_cast<Value *>(getUnderlyingObject(VConst, MaxLookup));
}
+/// Like getUnderlyingObject(), but will also look through phi/select nodes
+/// to establish a single underlying object.
+const Value *getUnderlyingObjectThroughPhisAndSelects(const Value *V,
+ unsigned MaxLookup = 6);
+
/// This method is similar to getUnderlyingObject except that it can
/// look through phi and select instructions and return multiple objects.
///
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index 1d54a66705a2a..cdb226a769074 100644
--- a/llvm/lib/Analysis/Loads.cpp
+++ b/llvm/lib/Analysis/Loads.cpp
@@ -743,9 +743,8 @@ static bool isPointerAlwaysReplaceable(const Value *From, const Value *To,
if (isa<Constant>(To) &&
isDereferenceablePointer(To, Type::getInt8Ty(To->getContext()), DL))
return true;
- if (getUnderlyingObject(From) == getUnderlyingObject(To))
- return true;
- return false;
+ return getUnderlyingObjectThroughPhisAndSelects(From) ==
+ getUnderlyingObjectThroughPhisAndSelects(To);
}
bool llvm::canReplacePointersInUseIfEqual(const Use &U, const Value *To,
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 535a248a5f1a2..369c821dd7335 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6609,6 +6609,48 @@ void llvm::getUnderlyingObjects(const Value *V,
} while (!Worklist.empty());
}
+const Value *
+llvm::getUnderlyingObjectThroughPhisAndSelects(const Value *V,
+ unsigned MaxLookup) {
+ const unsigned MaxVisited = 8;
+
+ SmallPtrSet<const Value *, 8> Visited;
+ SmallVector<const Value *, 8> Worklist;
+ Worklist.push_back(V);
+ const Value *Object = nullptr, *FirstObject = nullptr;
+ do {
+ const Value *P = Worklist.pop_back_val();
+ P = getUnderlyingObject(P, MaxLookup);
+
+ if (!FirstObject)
+ FirstObject = P;
+
+ if (!Visited.insert(P).second)
+ continue;
+
+ if (Visited.size() == MaxVisited)
+ return FirstObject;
+
+ if (auto *SI = dyn_cast<SelectInst>(P)) {
+ Worklist.push_back(SI->getTrueValue());
+ Worklist.push_back(SI->getFalseValue());
+ continue;
+ }
+
+ if (auto *PN = dyn_cast<PHINode>(P)) {
+ append_range(Worklist, PN->incoming_values());
+ continue;
+ }
+
+ if (!Object)
+ Object = P;
+ else if (Object != P)
+ return FirstObject;
+ } while (!Worklist.empty());
+
+ return Object;
+}
+
/// This is the function that does the work of looking through basic
/// ptrtoint+arithmetic+inttoptr sequences.
static const Value *getUnderlyingObjectFromInt(const Value *V) {
diff --git a/llvm/test/Transforms/GVN/condprop.ll b/llvm/test/Transforms/GVN/condprop.ll
index 02f4bb97d7ebd..3babdd8173a21 100644
--- a/llvm/test/Transforms/GVN/condprop.ll
+++ b/llvm/test/Transforms/GVN/condprop.ll
@@ -813,7 +813,7 @@ define void @select_same_obj(i1 %c, ptr %p, i64 %x) {
; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[EXIT:%.*]]
; CHECK: if:
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
-; CHECK-NEXT: call void @use_ptr(ptr [[P3]])
+; CHECK-NEXT: call void @use_ptr(ptr [[P]])
; CHECK-NEXT: ret void
; CHECK: exit:
; CHECK-NEXT: ret void
@@ -902,7 +902,7 @@ define void @phi_same_obj(i1 %c, ptr %p, i64 %x) {
; CHECK-NEXT: br i1 [[CMP]], label [[IF2:%.*]], label [[EXIT:%.*]]
; CHECK: if2:
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
-; CHECK-NEXT: call void @use_ptr(ptr [[P3]])
+; CHECK-NEXT: call void @use_ptr(ptr [[P]])
; CHECK-NEXT: ret void
; CHECK: exit:
; CHECK-NEXT: ret void
@@ -975,7 +975,7 @@ define void @phi_same_obj_cycle(i1 %c, ptr %p, i64 %x) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P_IV]], [[P]]
; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[LOOP_LATCH]]
; CHECK: if:
-; CHECK-NEXT: call void @use_ptr(ptr [[P_IV]])
+; CHECK-NEXT: call void @use_ptr(ptr [[P]])
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
; CHECK-NEXT: br label [[LOOP_LATCH]]
; CHECK: loop.latch:
>From 291ffd2f404980299966e025c93290554a4c5dd9 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Thu, 18 Jul 2024 18:17:42 +0200
Subject: [PATCH 2/4] Rename to getUnderlyingObjectAggressive
---
llvm/include/llvm/Analysis/ValueTracking.h | 8 ++++----
llvm/lib/Analysis/Loads.cpp | 4 ++--
llvm/lib/Analysis/ValueTracking.cpp | 5 ++---
3 files changed, 8 insertions(+), 9 deletions(-)
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index c689fd4851f12..f9be3f14392ab 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -736,10 +736,10 @@ inline Value *getUnderlyingObject(Value *V, unsigned MaxLookup = 6) {
return const_cast<Value *>(getUnderlyingObject(VConst, MaxLookup));
}
-/// Like getUnderlyingObject(), but will also look through phi/select nodes
-/// to establish a single underlying object.
-const Value *getUnderlyingObjectThroughPhisAndSelects(const Value *V,
- unsigned MaxLookup = 6);
+/// Like getUnderlyingObject(), but will try harder to find a single underlying
+/// object. In particular, this function also looks through selects and phis.
+const Value *getUnderlyingObjectAggressive(const Value *V,
+ unsigned MaxLookup = 6);
/// This method is similar to getUnderlyingObject except that it can
/// look through phi and select instructions and return multiple objects.
diff --git a/llvm/lib/Analysis/Loads.cpp b/llvm/lib/Analysis/Loads.cpp
index cdb226a769074..0dd4a3de08e5c 100644
--- a/llvm/lib/Analysis/Loads.cpp
+++ b/llvm/lib/Analysis/Loads.cpp
@@ -743,8 +743,8 @@ static bool isPointerAlwaysReplaceable(const Value *From, const Value *To,
if (isa<Constant>(To) &&
isDereferenceablePointer(To, Type::getInt8Ty(To->getContext()), DL))
return true;
- return getUnderlyingObjectThroughPhisAndSelects(From) ==
- getUnderlyingObjectThroughPhisAndSelects(To);
+ return getUnderlyingObjectAggressive(From) ==
+ getUnderlyingObjectAggressive(To);
}
bool llvm::canReplacePointersInUseIfEqual(const Use &U, const Value *To,
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 369c821dd7335..0559bbd0002c3 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6609,9 +6609,8 @@ void llvm::getUnderlyingObjects(const Value *V,
} while (!Worklist.empty());
}
-const Value *
-llvm::getUnderlyingObjectThroughPhisAndSelects(const Value *V,
- unsigned MaxLookup) {
+const Value *llvm::getUnderlyingObjectAggressive(const Value *V,
+ unsigned MaxLookup) {
const unsigned MaxVisited = 8;
SmallPtrSet<const Value *, 8> Visited;
>From 25857e83c7e80484599ceb7a199f5cc1e44765d6 Mon Sep 17 00:00:00 2001
From: Nikita Popov <nikita.ppv at gmail.com>
Date: Thu, 18 Jul 2024 21:13:17 +0200
Subject: [PATCH 3/4] Remove unused MaxLookup parameter
---
llvm/include/llvm/Analysis/ValueTracking.h | 3 +--
llvm/lib/Analysis/ValueTracking.cpp | 5 ++---
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/llvm/include/llvm/Analysis/ValueTracking.h b/llvm/include/llvm/Analysis/ValueTracking.h
index f9be3f14392ab..23bf06222d534 100644
--- a/llvm/include/llvm/Analysis/ValueTracking.h
+++ b/llvm/include/llvm/Analysis/ValueTracking.h
@@ -738,8 +738,7 @@ inline Value *getUnderlyingObject(Value *V, unsigned MaxLookup = 6) {
/// Like getUnderlyingObject(), but will try harder to find a single underlying
/// object. In particular, this function also looks through selects and phis.
-const Value *getUnderlyingObjectAggressive(const Value *V,
- unsigned MaxLookup = 6);
+const Value *getUnderlyingObjectAggressive(const Value *V);
/// This method is similar to getUnderlyingObject except that it can
/// look through phi and select instructions and return multiple objects.
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 0559bbd0002c3..245d00e0ec432 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6609,8 +6609,7 @@ void llvm::getUnderlyingObjects(const Value *V,
} while (!Worklist.empty());
}
-const Value *llvm::getUnderlyingObjectAggressive(const Value *V,
- unsigned MaxLookup) {
+const Value *llvm::getUnderlyingObjectAggressive(const Value *V) {
const unsigned MaxVisited = 8;
SmallPtrSet<const Value *, 8> Visited;
@@ -6619,7 +6618,7 @@ const Value *llvm::getUnderlyingObjectAggressive(const Value *V,
const Value *Object = nullptr, *FirstObject = nullptr;
do {
const Value *P = Worklist.pop_back_val();
- P = getUnderlyingObject(P, MaxLookup);
+ P = getUnderlyingObject(P);
if (!FirstObject)
FirstObject = P;
>From f160e9ca0887ac18e60890848a9909b62e54ffa2 Mon Sep 17 00:00:00 2001
From: Nikita Popov <npopov at redhat.com>
Date: Mon, 22 Jul 2024 11:08:03 +0200
Subject: [PATCH 4/4] Refactor FirstObject logic
---
llvm/lib/Analysis/ValueTracking.cpp | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 245d00e0ec432..bb367dd2d9a73 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -6615,19 +6615,18 @@ const Value *llvm::getUnderlyingObjectAggressive(const Value *V) {
SmallPtrSet<const Value *, 8> Visited;
SmallVector<const Value *, 8> Worklist;
Worklist.push_back(V);
- const Value *Object = nullptr, *FirstObject = nullptr;
+ const Value *Object = nullptr;
do {
const Value *P = Worklist.pop_back_val();
P = getUnderlyingObject(P);
- if (!FirstObject)
- FirstObject = P;
-
if (!Visited.insert(P).second)
continue;
- if (Visited.size() == MaxVisited)
- return FirstObject;
+ if (Visited.size() == MaxVisited) {
+ Object = nullptr;
+ break;
+ }
if (auto *SI = dyn_cast<SelectInst>(P)) {
Worklist.push_back(SI->getTrueValue());
@@ -6642,10 +6641,16 @@ const Value *llvm::getUnderlyingObjectAggressive(const Value *V) {
if (!Object)
Object = P;
- else if (Object != P)
- return FirstObject;
+ else if (Object != P) {
+ Object = nullptr;
+ break;
+ }
} while (!Worklist.empty());
+ // If we tried looking through phi/select but did not end up with a single
+ // underlying object, fall back to the non-recursive underlying object of V.
+ if (!Object)
+ return getUnderlyingObject(V);
return Object;
}
More information about the llvm-commits
mailing list