From f303e010356633b50f9631528f30bee7747ca720 Mon Sep 17 00:00:00 2001 From: Yingwei Zheng 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 0000000000000..ce91541b57679 --- /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 0000000000000..b9dd80d032c30 --- /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 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 2ba600497e00d..ad9b1217089d7 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(V1)) + if (auto *CB2 = dyn_cast(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(nullptr), P)); PREPred = P; ++NumWithout; diff --git a/llvm/lib/Transforms/Scalar/NewGVN.cpp b/llvm/lib/Transforms/Scalar/NewGVN.cpp index 13d9e8f186b47..6800ad51cc0a8 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(V1)) + if (auto *CB2 = dyn_cast(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(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())) + if (!match(DefI, m_Intrinsic())) { + 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(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 65c1669f92b4d..47a7049255961 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(I)) ReplInst->andIRFlags(I); + // Handle attributes. + if (auto *CB1 = dyn_cast(ReplInst)) { + if (auto *CB2 = dyn_cast(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 ce91541b57679..35e73b1a4439b 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 b9dd80d032c30..a919c8c304b1b 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:.*]]