[llvm] [GVN] Look through select/phi when determining underlying object (PR #99509)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 18 08:02:56 PDT 2024
https://github.com/nikic created https://github.com/llvm/llvm-project/pull/99509
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.
>From bbbfa04cc9b3fcfd9c24c03afcd4e164fdc0ed1b 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] [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:
More information about the llvm-commits
mailing list