Skip to content

Commit bbbfa04

Browse files
committed
[GVN] Look through select/phi when determining underlying object
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 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.
1 parent 9711f6b commit bbbfa04

File tree

4 files changed

+52
-6
lines changed

4 files changed

+52
-6
lines changed

llvm/include/llvm/Analysis/ValueTracking.h

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

739+
/// Like getUnderlyingObject(), but will also look through phi/select nodes
740+
/// to establish a single underlying object.
741+
const Value *getUnderlyingObjectThroughPhisAndSelects(const Value *V,
742+
unsigned MaxLookup = 6);
743+
739744
/// This method is similar to getUnderlyingObject except that it can
740745
/// look through phi and select instructions and return multiple objects.
741746
///

llvm/lib/Analysis/Loads.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -743,9 +743,8 @@ static bool isPointerAlwaysReplaceable(const Value *From, const Value *To,
743743
if (isa<Constant>(To) &&
744744
isDereferenceablePointer(To, Type::getInt8Ty(To->getContext()), DL))
745745
return true;
746-
if (getUnderlyingObject(From) == getUnderlyingObject(To))
747-
return true;
748-
return false;
746+
return getUnderlyingObjectThroughPhisAndSelects(From) ==
747+
getUnderlyingObjectThroughPhisAndSelects(To);
749748
}
750749

751750
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
@@ -6609,6 +6609,48 @@ void llvm::getUnderlyingObjects(const Value *V,
66096609
} while (!Worklist.empty());
66106610
}
66116611

6612+
const Value *
6613+
llvm::getUnderlyingObjectThroughPhisAndSelects(const Value *V,
6614+
unsigned MaxLookup) {
6615+
const unsigned MaxVisited = 8;
6616+
6617+
SmallPtrSet<const Value *, 8> Visited;
6618+
SmallVector<const Value *, 8> Worklist;
6619+
Worklist.push_back(V);
6620+
const Value *Object = nullptr, *FirstObject = nullptr;
6621+
do {
6622+
const Value *P = Worklist.pop_back_val();
6623+
P = getUnderlyingObject(P, MaxLookup);
6624+
6625+
if (!FirstObject)
6626+
FirstObject = P;
6627+
6628+
if (!Visited.insert(P).second)
6629+
continue;
6630+
6631+
if (Visited.size() == MaxVisited)
6632+
return FirstObject;
6633+
6634+
if (auto *SI = dyn_cast<SelectInst>(P)) {
6635+
Worklist.push_back(SI->getTrueValue());
6636+
Worklist.push_back(SI->getFalseValue());
6637+
continue;
6638+
}
6639+
6640+
if (auto *PN = dyn_cast<PHINode>(P)) {
6641+
append_range(Worklist, PN->incoming_values());
6642+
continue;
6643+
}
6644+
6645+
if (!Object)
6646+
Object = P;
6647+
else if (Object != P)
6648+
return FirstObject;
6649+
} while (!Worklist.empty());
6650+
6651+
return Object;
6652+
}
6653+
66126654
/// This is the function that does the work of looking through basic
66136655
/// ptrtoint+arithmetic+inttoptr sequences.
66146656
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)