[llvm] [GVN][NewGVN][Local] Handle attributes for function calls after CSE (PR #114011)
Yingwei Zheng via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 29 00:32:24 PDT 2024
https://github.com/dtcxzyw created https://github.com/llvm/llvm-project/pull/114011
This patch intersects attributes of two calls to avoid introducing UB. It also skips incompatible call pairs in GVN/NewGVN. However, I cannot provide negative tests for these changes.
Fixes https://github.com/llvm/llvm-project/issues/113997.
>From f303e010356633b50f9631528f30bee7747ca720 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 29 Oct 2024 15:10:01 +0800
Subject: [PATCH 1/2] [GVN][NewGVN] Add pre-commit tests. NFC.
---
llvm/test/Transforms/GVN/pr113997.ll | 33 +++++++++++++++++++++++++
llvm/test/Transforms/NewGVN/pr113997.ll | 33 +++++++++++++++++++++++++
2 files changed, 66 insertions(+)
create mode 100644 llvm/test/Transforms/GVN/pr113997.ll
create mode 100644 llvm/test/Transforms/NewGVN/pr113997.ll
diff --git a/llvm/test/Transforms/GVN/pr113997.ll b/llvm/test/Transforms/GVN/pr113997.ll
new file mode 100644
index 00000000000000..ce91541b57679c
--- /dev/null
+++ b/llvm/test/Transforms/GVN/pr113997.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=gvn < %s | FileCheck %s
+
+; Make sure attributes in function calls are intersected correctly.
+
+define i1 @bucket(i32 noundef %x) {
+; CHECK-LABEL: define i1 @bucket(
+; CHECK-SAME: i32 noundef [[X:%.*]]) {
+; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
+; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 [[X]])
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1]], 2
+; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i1 [[CMP2]], i1 false
+; CHECK-NEXT: br i1 [[COND]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK: [[IF_ELSE]]:
+; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP1]], 1
+; CHECK-NEXT: ret i1 [[RES]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: ret i1 false
+;
+ %cmp1 = icmp sgt i32 %x, 0
+ %ctpop1 = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 %x)
+ %cmp2 = icmp samesign ult i32 %ctpop1, 2
+ %cond = select i1 %cmp1, i1 %cmp2, i1 false
+ br i1 %cond, label %if.then, label %if.else
+
+if.else:
+ %ctpop2 = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 %x)
+ %res = icmp eq i32 %ctpop2, 1
+ ret i1 %res
+
+if.then:
+ ret i1 false
+}
diff --git a/llvm/test/Transforms/NewGVN/pr113997.ll b/llvm/test/Transforms/NewGVN/pr113997.ll
new file mode 100644
index 00000000000000..b9dd80d032c30c
--- /dev/null
+++ b/llvm/test/Transforms/NewGVN/pr113997.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
+; RUN: opt -S -passes=newgvn < %s | FileCheck %s
+
+; Make sure attributes in function calls are intersected correctly.
+
+define i1 @bucket(i32 noundef %x) {
+; CHECK-LABEL: define i1 @bucket(
+; CHECK-SAME: i32 noundef [[X:%.*]]) {
+; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
+; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 [[X]])
+; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1]], 2
+; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i1 [[CMP2]], i1 false
+; CHECK-NEXT: br i1 [[COND]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
+; CHECK: [[IF_ELSE]]:
+; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP1]], 1
+; CHECK-NEXT: ret i1 [[RES]]
+; CHECK: [[IF_THEN]]:
+; CHECK-NEXT: ret i1 false
+;
+ %cmp1 = icmp sgt i32 %x, 0
+ %ctpop1 = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 %x)
+ %cmp2 = icmp samesign ult i32 %ctpop1, 2
+ %cond = select i1 %cmp1, i1 %cmp2, i1 false
+ br i1 %cond, label %if.then, label %if.else
+
+if.else:
+ %ctpop2 = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 %x)
+ %res = icmp eq i32 %ctpop2, 1
+ ret i1 %res
+
+if.then:
+ ret i1 false
+}
>From fe37aad5b370f96fe2378850005b807685cbaab0 Mon Sep 17 00:00:00 2001
From: Yingwei Zheng <dtcxzyw2333 at gmail.com>
Date: Tue, 29 Oct 2024 15:17:28 +0800
Subject: [PATCH 2/2] [GVN][NewGVN][Local] Handle attributes for function calls
after CSE
---
llvm/lib/Transforms/Scalar/GVN.cpp | 14 ++++++++++--
llvm/lib/Transforms/Scalar/NewGVN.cpp | 30 ++++++++++++++++++++-----
llvm/lib/Transforms/Utils/Local.cpp | 11 +++++++++
llvm/test/Transforms/GVN/pr113997.ll | 2 +-
llvm/test/Transforms/NewGVN/pr113997.ll | 2 +-
5 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp
index 2ba600497e00d3..ad9b1217089d7d 100644
--- a/llvm/lib/Transforms/Scalar/GVN.cpp
+++ b/llvm/lib/Transforms/Scalar/GVN.cpp
@@ -2189,6 +2189,16 @@ bool GVNPass::processAssumeIntrinsic(AssumeInst *IntrinsicI) {
return Changed;
}
+// Return true iff V1 can be replaced with V2.
+static bool canBeReplacedBy(Value *V1, Value *V2) {
+ if (auto *CB1 = dyn_cast<CallBase>(V1))
+ if (auto *CB2 = dyn_cast<CallBase>(V2))
+ return CB1->getAttributes()
+ .intersectWith(CB2->getContext(), CB2->getAttributes())
+ .has_value();
+ return true;
+}
+
static void patchAndReplaceAllUsesWith(Instruction *I, Value *Repl) {
patchReplacementInstruction(I, Repl);
I->replaceAllUsesWith(Repl);
@@ -2734,7 +2744,7 @@ bool GVNPass::processInstruction(Instruction *I) {
// Perform fast-path value-number based elimination of values inherited from
// dominators.
Value *Repl = findLeader(I->getParent(), Num);
- if (!Repl) {
+ if (!Repl || !canBeReplacedBy(I, Repl)) {
// Failure, just remember this instance for future use.
LeaderTable.insert(Num, I, I->getParent());
return false;
@@ -3000,7 +3010,7 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) {
uint32_t TValNo = VN.phiTranslate(P, CurrentBlock, ValNo, *this);
Value *predV = findLeader(P, TValNo);
- if (!predV) {
+ if (!predV || !canBeReplacedBy(CurInst, predV)) {
predMap.push_back(std::make_pair(static_cast<Value *>(nullptr), P));
PREPred = P;
++NumWithout;
diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp
index 13d9e8f186b47c..6800ad51cc0a8f 100644
--- a/llvm/lib/Transforms/Scalar/NewGVN.cpp
+++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp
@@ -3854,6 +3854,16 @@ Value *NewGVN::findPHIOfOpsLeader(const Expression *E,
return nullptr;
}
+// Return true iff V1 can be replaced with V2.
+static bool canBeReplacedBy(Value *V1, Value *V2) {
+ if (auto *CB1 = dyn_cast<CallBase>(V1))
+ if (auto *CB2 = dyn_cast<CallBase>(V2))
+ return CB1->getAttributes()
+ .intersectWith(CB2->getContext(), CB2->getAttributes())
+ .has_value();
+ return true;
+}
+
bool NewGVN::eliminateInstructions(Function &F) {
// This is a non-standard eliminator. The normal way to eliminate is
// to walk the dominator tree in order, keeping track of available
@@ -3963,6 +3973,9 @@ bool NewGVN::eliminateInstructions(Function &F) {
MembersLeft.insert(Member);
continue;
}
+ if (!canBeReplacedBy(Member, Leader))
+ continue;
+
LLVM_DEBUG(dbgs() << "Found replacement " << *(Leader) << " for "
<< *Member << "\n");
auto *I = cast<Instruction>(Member);
@@ -4069,8 +4082,11 @@ bool NewGVN::eliminateInstructions(Function &F) {
if (DominatingLeader != Def) {
// Even if the instruction is removed, we still need to update
// flags/metadata due to downstreams users of the leader.
- if (!match(DefI, m_Intrinsic<Intrinsic::ssa_copy>()))
+ if (!match(DefI, m_Intrinsic<Intrinsic::ssa_copy>())) {
+ if (!canBeReplacedBy(DefI, DominatingLeader))
+ continue;
patchReplacementInstruction(DefI, DominatingLeader);
+ }
markInstructionForDeletion(DefI);
}
@@ -4112,17 +4128,21 @@ bool NewGVN::eliminateInstructions(Function &F) {
// Don't replace our existing users with ourselves.
if (U->get() == DominatingLeader)
continue;
- LLVM_DEBUG(dbgs()
- << "Found replacement " << *DominatingLeader << " for "
- << *U->get() << " in " << *(U->getUser()) << "\n");
// If we replaced something in an instruction, handle the patching of
// metadata. Skip this if we are replacing predicateinfo with its
// original operand, as we already know we can just drop it.
auto *ReplacedInst = cast<Instruction>(U->get());
auto *PI = PredInfo->getPredicateInfoFor(ReplacedInst);
- if (!PI || DominatingLeader != PI->OriginalOp)
+ if (!PI || DominatingLeader != PI->OriginalOp) {
+ if (!canBeReplacedBy(ReplacedInst, DominatingLeader))
+ continue;
patchReplacementInstruction(ReplacedInst, DominatingLeader);
+ }
+
+ LLVM_DEBUG(dbgs()
+ << "Found replacement " << *DominatingLeader << " for "
+ << *U->get() << " in " << *(U->getUser()) << "\n");
U->set(DominatingLeader);
// This is now a use of the dominating leader, which means if the
// dominating leader was dead, it's now live!
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index 65c1669f92b4d3..47a70492559610 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -3508,6 +3508,17 @@ void llvm::patchReplacementInstruction(Instruction *I, Value *Repl) {
else if (!isa<LoadInst>(I))
ReplInst->andIRFlags(I);
+ // Handle attributes.
+ if (auto *CB1 = dyn_cast<CallBase>(ReplInst)) {
+ if (auto *CB2 = dyn_cast<CallBase>(I)) {
+ bool Success = CB1->tryIntersectAttributes(CB2);
+ assert(Success && "We should not be trying to sink callbases "
+ "with non-intersectable attributes");
+ // For NDEBUG Compile.
+ (void)Success;
+ }
+ }
+
// FIXME: If both the original and replacement value are part of the
// same control-flow region (meaning that the execution of one
// guarantees the execution of the other), then we can combine the
diff --git a/llvm/test/Transforms/GVN/pr113997.ll b/llvm/test/Transforms/GVN/pr113997.ll
index ce91541b57679c..35e73b1a4439b3 100644
--- a/llvm/test/Transforms/GVN/pr113997.ll
+++ b/llvm/test/Transforms/GVN/pr113997.ll
@@ -7,7 +7,7 @@ define i1 @bucket(i32 noundef %x) {
; CHECK-LABEL: define i1 @bucket(
; CHECK-SAME: i32 noundef [[X:%.*]]) {
; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
-; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 [[X]])
+; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[X]])
; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1]], 2
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i1 [[CMP2]], i1 false
; CHECK-NEXT: br i1 [[COND]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
diff --git a/llvm/test/Transforms/NewGVN/pr113997.ll b/llvm/test/Transforms/NewGVN/pr113997.ll
index b9dd80d032c30c..a919c8c304b1b9 100644
--- a/llvm/test/Transforms/NewGVN/pr113997.ll
+++ b/llvm/test/Transforms/NewGVN/pr113997.ll
@@ -7,7 +7,7 @@ define i1 @bucket(i32 noundef %x) {
; CHECK-LABEL: define i1 @bucket(
; CHECK-SAME: i32 noundef [[X:%.*]]) {
; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i32 [[X]], 0
-; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 1, 32) i32 @llvm.ctpop.i32(i32 [[X]])
+; CHECK-NEXT: [[CTPOP1:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[X]])
; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1]], 2
; CHECK-NEXT: [[COND:%.*]] = select i1 [[CMP1]], i1 [[CMP2]], i1 false
; CHECK-NEXT: br i1 [[COND]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]]
More information about the llvm-commits
mailing list