-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[GVN] Handle provenance when propagating assume equality #151953
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-transforms Author: Nikita Popov (nikic) ChangesIf we have a known Call canReplacePointersInUseIfEqual() before performing the replacement. This is subject to the usual approximations (e.g. that we always allow replacement with a dereferenceable constant and null). This restriction does not appear to have any impact in practice. Full diff: https://github.com/llvm/llvm-project/pull/151953.diff 3 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index fa6ee95d33d10..d6f55bbea7abe 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2499,11 +2499,11 @@ void GVNPass::assignBlockRPONumber(Function &F) {
bool GVNPass::replaceOperandsForInBlockEquality(Instruction *Instr) const {
bool Changed = false;
for (unsigned OpNum = 0; OpNum < Instr->getNumOperands(); ++OpNum) {
- Value *Operand = Instr->getOperand(OpNum);
- auto It = ReplaceOperandsWithMap.find(Operand);
+ Use &Operand = Instr->getOperandUse(OpNum);
+ auto It = ReplaceOperandsWithMap.find(Operand.get());
if (It != ReplaceOperandsWithMap.end()) {
- // Do not replace lifetime alloca argument with something else.
- if (Instr->isLifetimeStartOrEnd())
+ const DataLayout &DL = Instr->getDataLayout();
+ if (!canReplacePointersInUseIfEqual(Operand, It->second, DL))
continue;
LLVM_DEBUG(dbgs() << "GVN replacing: " << *Operand << " with "
diff --git a/llvm/test/Transforms/GVN/assume-equal.ll b/llvm/test/Transforms/GVN/assume-equal.ll
index a27b0e45bf9e1..0c922daf82b32 100644
--- a/llvm/test/Transforms/GVN/assume-equal.ll
+++ b/llvm/test/Transforms/GVN/assume-equal.ll
@@ -343,6 +343,50 @@ meh:
ret i1 %k
}
+define i8 @assume_ptr_eq_different_prov_matters(ptr %p, ptr %p2) {
+; CHECK-LABEL: define i8 @assume_ptr_eq_different_prov_matters(
+; CHECK-SAME: ptr [[P:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P]], [[P2]]
+; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT: [[V:%.*]] = load i8, ptr [[P2]], align 1
+; CHECK-NEXT: ret i8 [[V]]
+;
+ %cmp = icmp eq ptr %p, %p2
+ call void @llvm.assume(i1 %cmp)
+ %v = load i8, ptr %p2
+ ret i8 %v
+}
+
+define i1 @assume_ptr_eq_different_prov_does_not_matter(ptr %p, ptr %p2) {
+; CHECK-LABEL: define i1 @assume_ptr_eq_different_prov_does_not_matter(
+; CHECK-SAME: ptr [[P:%.*]], ptr [[P2:%.*]]) {
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P]], [[P2]]
+; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT: [[C:%.*]] = icmp eq ptr [[P]], null
+; CHECK-NEXT: ret i1 [[C]]
+;
+ %cmp = icmp eq ptr %p, %p2
+ call void @llvm.assume(i1 %cmp)
+ %c = icmp eq ptr %p2, null
+ ret i1 %c
+}
+
+define i8 @assume_ptr_eq_same_prov(ptr %p, i64 %x) {
+; CHECK-LABEL: define i8 @assume_ptr_eq_same_prov(
+; CHECK-SAME: ptr [[P:%.*]], i64 [[X:%.*]]) {
+; CHECK-NEXT: [[P2:%.*]] = getelementptr i8, ptr [[P]], i64 [[X]]
+; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[P]], [[P2]]
+; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
+; CHECK-NEXT: [[V:%.*]] = load i8, ptr [[P]], align 1
+; CHECK-NEXT: ret i8 [[V]]
+;
+ %p2 = getelementptr i8, ptr %p, i64 %x
+ %cmp = icmp eq ptr %p, %p2
+ call void @llvm.assume(i1 %cmp)
+ %v = load i8, ptr %p2
+ ret i8 %v
+}
+
declare noalias ptr @_Znwm(i64)
declare void @_ZN1AC1Ev(ptr)
declare void @llvm.assume(i1)
diff --git a/llvm/test/Transforms/GVN/lifetime-simple.ll b/llvm/test/Transforms/GVN/lifetime-simple.ll
index 30883bd9dc6d3..89ca127a47fda 100644
--- a/llvm/test/Transforms/GVN/lifetime-simple.ll
+++ b/llvm/test/Transforms/GVN/lifetime-simple.ll
@@ -29,7 +29,7 @@ define void @assume_eq_arg(ptr %arg) {
; CHECK-NEXT: [[CMP:%.*]] = icmp eq ptr [[ALLOCA]], [[ARG]]
; CHECK-NEXT: call void @llvm.assume(i1 [[CMP]])
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 4, ptr [[ALLOCA]])
-; CHECK-NEXT: store volatile i32 0, ptr [[ARG]], align 4
+; CHECK-NEXT: store volatile i32 0, ptr [[ALLOCA]], align 4
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 4, ptr [[ALLOCA]])
; CHECK-NEXT: ret void
;
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
If we have a known
p == p2
equality, we cannot replacep2
withp
unless they are known to have the same provenance. GVN handles this when propagating equalities from conditions, but not for assumes, as these go through a different code path for uses in the same block.Call canReplacePointersInUseIfEqual() before performing the replacement. This is subject to the usual approximations (e.g. that we always allow replacement with a dereferenceable constant and null).
This restriction does not appear to have any impact in practice.