Skip to content

Commit 32cd189

Browse files
authored
[GVN] Look through select/phi when determining underlying object (#99509)
This addresses an optimization regression in Rust we have observed after #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 getUnderlyingObjectAggressive() 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.
1 parent 091ec15 commit 32cd189

File tree

4 files changed

+51
-6
lines changed

4 files changed

+51
-6
lines changed

llvm/include/llvm/Analysis/ValueTracking.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,6 +736,10 @@ inline Value *getUnderlyingObject(Value *V, unsigned MaxLookup = 6) {
736736
return const_cast<Value *>(getUnderlyingObject(VConst, MaxLookup));
737737
}
738738

739+
/// Like getUnderlyingObject(), but will try harder to find a single underlying
740+
/// object. In particular, this function also looks through selects and phis.
741+
const Value *getUnderlyingObjectAggressive(const Value *V);
742+
739743
/// This method is similar to getUnderlyingObject except that it can
740744
/// look through phi and select instructions and return multiple objects.
741745
///

llvm/lib/Analysis/Loads.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -750,9 +750,8 @@ static bool isPointerAlwaysReplaceable(const Value *From, const Value *To,
750750
if (isa<Constant>(To) &&
751751
isDereferenceablePointer(To, Type::getInt8Ty(To->getContext()), DL))
752752
return true;
753-
if (getUnderlyingObject(From) == getUnderlyingObject(To))
754-
return true;
755-
return false;
753+
return getUnderlyingObjectAggressive(From) ==
754+
getUnderlyingObjectAggressive(To);
756755
}
757756

758757
bool llvm::canReplacePointersInUseIfEqual(const Use &U, const Value *To,

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6620,6 +6620,48 @@ void llvm::getUnderlyingObjects(const Value *V,
66206620
} while (!Worklist.empty());
66216621
}
66226622

6623+
const Value *llvm::getUnderlyingObjectAggressive(const Value *V) {
6624+
const unsigned MaxVisited = 8;
6625+
6626+
SmallPtrSet<const Value *, 8> Visited;
6627+
SmallVector<const Value *, 8> Worklist;
6628+
Worklist.push_back(V);
6629+
const Value *Object = nullptr;
6630+
// Used as fallback if we can't find a common underlying object through
6631+
// recursion.
6632+
bool First = true;
6633+
const Value *FirstObject = getUnderlyingObject(V);
6634+
do {
6635+
const Value *P = Worklist.pop_back_val();
6636+
P = First ? FirstObject : getUnderlyingObject(P);
6637+
First = false;
6638+
6639+
if (!Visited.insert(P).second)
6640+
continue;
6641+
6642+
if (Visited.size() == MaxVisited)
6643+
return FirstObject;
6644+
6645+
if (auto *SI = dyn_cast<SelectInst>(P)) {
6646+
Worklist.push_back(SI->getTrueValue());
6647+
Worklist.push_back(SI->getFalseValue());
6648+
continue;
6649+
}
6650+
6651+
if (auto *PN = dyn_cast<PHINode>(P)) {
6652+
append_range(Worklist, PN->incoming_values());
6653+
continue;
6654+
}
6655+
6656+
if (!Object)
6657+
Object = P;
6658+
else if (Object != P)
6659+
return FirstObject;
6660+
} while (!Worklist.empty());
6661+
6662+
return Object;
6663+
}
6664+
66236665
/// This is the function that does the work of looking through basic
66246666
/// ptrtoint+arithmetic+inttoptr sequences.
66256667
static const Value *getUnderlyingObjectFromInt(const Value *V) {

llvm/test/Transforms/GVN/condprop.ll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -813,7 +813,7 @@ define void @select_same_obj(i1 %c, ptr %p, i64 %x) {
813813
; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[EXIT:%.*]]
814814
; CHECK: if:
815815
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
816-
; CHECK-NEXT: call void @use_ptr(ptr [[P3]])
816+
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
817817
; CHECK-NEXT: ret void
818818
; CHECK: exit:
819819
; CHECK-NEXT: ret void
@@ -902,7 +902,7 @@ define void @phi_same_obj(i1 %c, ptr %p, i64 %x) {
902902
; CHECK-NEXT: br i1 [[CMP]], label [[IF2:%.*]], label [[EXIT:%.*]]
903903
; CHECK: if2:
904904
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
905-
; CHECK-NEXT: call void @use_ptr(ptr [[P3]])
905+
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
906906
; CHECK-NEXT: ret void
907907
; CHECK: exit:
908908
; CHECK-NEXT: ret void
@@ -975,7 +975,7 @@ define void @phi_same_obj_cycle(i1 %c, ptr %p, i64 %x) {
975975
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P_IV]], [[P]]
976976
; CHECK-NEXT: br i1 [[CMP]], label [[IF:%.*]], label [[LOOP_LATCH]]
977977
; CHECK: if:
978-
; CHECK-NEXT: call void @use_ptr(ptr [[P_IV]])
978+
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
979979
; CHECK-NEXT: call void @use_ptr(ptr [[P]])
980980
; CHECK-NEXT: br label [[LOOP_LATCH]]
981981
; CHECK: loop.latch:

0 commit comments

Comments
 (0)