From 155fed8017812fe451720f79d34c8a7287c9a24d Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Fri, 1 Nov 2024 22:30:19 +0800 Subject: [PATCH 1/4] [GVN][NewGVN] Add pre-commit tests. NFC. --- llvm/test/Transforms/GVN/pr113997.ll | 33 +++++++++++++++++++++++++ llvm/test/Transforms/NewGVN/pr113997.ll | 32 ++++++++++++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/llvm/test/Transforms/GVN/pr113997.ll b/llvm/test/Transforms/GVN/pr113997.ll index 35e73b1a4439b..8f9ef3776a400 100644 --- a/llvm/test/Transforms/GVN/pr113997.ll +++ b/llvm/test/Transforms/GVN/pr113997.ll @@ -31,3 +31,36 @@ if.else: if.then: ret i1 false } + +define i1 @bucket2(i32 noundef %x) { +; CHECK-LABEL: define i1 @bucket2( +; 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 zeroext [[X]]) +; CHECK-NEXT: [[CTPOP1INC:%.*]] = add i32 [[CTPOP1]], 1 +; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1INC]], 3 +; 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: [[CTPOP2:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[X]]) +; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP1INC]], 2 +; 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 zeroext %x) + %ctpop1inc = add i32 %ctpop1, 1 + %cmp2 = icmp samesign ult i32 %ctpop1inc, 3 + %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) + %ctpop2inc = add i32 %ctpop2, 1 + %res = icmp eq i32 %ctpop2inc, 2 + ret i1 %res + +if.then: + ret i1 false +} diff --git a/llvm/test/Transforms/NewGVN/pr113997.ll b/llvm/test/Transforms/NewGVN/pr113997.ll index a919c8c304b1b..8761af87c9891 100644 --- a/llvm/test/Transforms/NewGVN/pr113997.ll +++ b/llvm/test/Transforms/NewGVN/pr113997.ll @@ -31,3 +31,35 @@ if.else: if.then: ret i1 false } + +define i1 @bucket2(i32 noundef %x) { +; CHECK-LABEL: define i1 @bucket2( +; 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 zeroext [[X]]) +; CHECK-NEXT: [[CTPOP1INC:%.*]] = add i32 [[CTPOP1]], 1 +; CHECK-NEXT: [[CMP2:%.*]] = icmp samesign ult i32 [[CTPOP1INC]], 3 +; 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 [[CTPOP1INC]], 2 +; 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 zeroext %x) + %ctpop1inc = add i32 %ctpop1, 1 + %cmp2 = icmp samesign ult i32 %ctpop1inc, 3 + %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) + %ctpop2inc = add i32 %ctpop2, 1 + %res = icmp eq i32 %ctpop2inc, 2 + ret i1 %res + +if.then: + ret i1 false +} From 468916f75e8ee3f011b3579b6c6175bdd0d8b485 Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Fri, 1 Nov 2024 22:50:50 +0800 Subject: [PATCH 2/4] [GVN][NewGVN] Take call attributes into account in expressions --- .../llvm/Transforms/Scalar/GVNExpression.h | 6 ++++ llvm/lib/Transforms/Scalar/GVN.cpp | 23 ++++++------- llvm/lib/Transforms/Scalar/NewGVN.cpp | 34 ++++++++----------- llvm/test/Transforms/GVN/pr113997.ll | 5 ++- llvm/test/Transforms/NewGVN/pr113997.ll | 6 +++- 5 files changed, 40 insertions(+), 34 deletions(-) diff --git a/llvm/include/llvm/Transforms/Scalar/GVNExpression.h b/llvm/include/llvm/Transforms/Scalar/GVNExpression.h index 2433890d0df80..71437953640c9 100644 --- a/llvm/include/llvm/Transforms/Scalar/GVNExpression.h +++ b/llvm/include/llvm/Transforms/Scalar/GVNExpression.h @@ -315,6 +315,12 @@ class CallExpression final : public MemoryExpression { return EB->getExpressionType() == ET_Call; } + bool equals(const Expression &Other) const override; + bool exactlyEquals(const Expression &Other) const override { + return Expression::exactlyEquals(Other) && + cast(Other).Call == Call; + } + // Debugging support void printInternal(raw_ostream &OS, bool PrintEType) const override { if (PrintEType) diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index ad9b1217089d7..4dfbc73f3539a 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -143,6 +143,8 @@ struct llvm::GVNPass::Expression { Type *type = nullptr; SmallVector varargs; + std::optional attrs; + Expression(uint32_t o = ~2U) : opcode(o) {} bool operator==(const Expression &other) const { @@ -154,6 +156,11 @@ struct llvm::GVNPass::Expression { return false; if (varargs != other.varargs) return false; + if (attrs.has_value() != other.attrs.has_value()) + return false; + if (attrs.has_value() && + !attrs->intersectWith(type->getContext(), *other.attrs).has_value()) + return false; return true; } @@ -364,6 +371,8 @@ GVNPass::Expression GVNPass::ValueTable::createExpr(Instruction *I) { } else if (auto *SVI = dyn_cast(I)) { ArrayRef ShuffleMask = SVI->getShuffleMask(); e.varargs.append(ShuffleMask.begin(), ShuffleMask.end()); + } else if (auto *CB = dyn_cast(I)) { + e.attrs = CB->getAttributes(); } return e; @@ -2189,16 +2198,6 @@ 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); @@ -2744,7 +2743,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 || !canBeReplacedBy(I, Repl)) { + if (!Repl) { // Failure, just remember this instance for future use. LeaderTable.insert(Num, I, I->getParent()); return false; @@ -3010,7 +3009,7 @@ bool GVNPass::performScalarPRE(Instruction *CurInst) { uint32_t TValNo = VN.phiTranslate(P, CurrentBlock, ValNo, *this); Value *predV = findLeader(P, TValNo); - if (!predV || !canBeReplacedBy(CurInst, predV)) { + if (!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 6800ad51cc0a8..0cba8739441bc 100644 --- a/llvm/lib/Transforms/Scalar/NewGVN.cpp +++ b/llvm/lib/Transforms/Scalar/NewGVN.cpp @@ -945,6 +945,18 @@ bool StoreExpression::equals(const Expression &Other) const { return true; } +bool CallExpression::equals(const Expression &Other) const { + if (!MemoryExpression::equals(Other)) + return false; + + if (auto *RHS = dyn_cast(&Other)) + return Call->getAttributes() + .intersectWith(Call->getContext(), RHS->Call->getAttributes()) + .has_value(); + + return false; +} + // Determine if the edge From->To is a backedge bool NewGVN::isBackedge(BasicBlock *From, BasicBlock *To) const { return From == To || @@ -3854,16 +3866,6 @@ 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 @@ -3973,8 +3975,6 @@ bool NewGVN::eliminateInstructions(Function &F) { MembersLeft.insert(Member); continue; } - if (!canBeReplacedBy(Member, Leader)) - continue; LLVM_DEBUG(dbgs() << "Found replacement " << *(Leader) << " for " << *Member << "\n"); @@ -4082,11 +4082,8 @@ 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 (!canBeReplacedBy(DefI, DominatingLeader)) - continue; + if (!match(DefI, m_Intrinsic())) patchReplacementInstruction(DefI, DominatingLeader); - } markInstructionForDeletion(DefI); } @@ -4134,11 +4131,8 @@ bool NewGVN::eliminateInstructions(Function &F) { // 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 (!canBeReplacedBy(ReplacedInst, DominatingLeader)) - continue; + if (!PI || DominatingLeader != PI->OriginalOp) patchReplacementInstruction(ReplacedInst, DominatingLeader); - } LLVM_DEBUG(dbgs() << "Found replacement " << *DominatingLeader << " for " diff --git a/llvm/test/Transforms/GVN/pr113997.ll b/llvm/test/Transforms/GVN/pr113997.ll index 8f9ef3776a400..6aebdb3a5dfac 100644 --- a/llvm/test/Transforms/GVN/pr113997.ll +++ b/llvm/test/Transforms/GVN/pr113997.ll @@ -32,6 +32,8 @@ if.then: ret i1 false } +; Make sure we don't merge these two users of the incompatible call pair. + define i1 @bucket2(i32 noundef %x) { ; CHECK-LABEL: define i1 @bucket2( ; CHECK-SAME: i32 noundef [[X:%.*]]) { @@ -43,7 +45,8 @@ define i1 @bucket2(i32 noundef %x) { ; CHECK-NEXT: br i1 [[COND]], label %[[IF_THEN:.*]], label %[[IF_ELSE:.*]] ; CHECK: [[IF_ELSE]]: ; CHECK-NEXT: [[CTPOP2:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[X]]) -; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP1INC]], 2 +; CHECK-NEXT: [[CTPOP2INC:%.*]] = add i32 [[CTPOP2]], 1 +; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP2INC]], 2 ; CHECK-NEXT: ret i1 [[RES]] ; CHECK: [[IF_THEN]]: ; CHECK-NEXT: ret i1 false diff --git a/llvm/test/Transforms/NewGVN/pr113997.ll b/llvm/test/Transforms/NewGVN/pr113997.ll index 8761af87c9891..59dce09e89c88 100644 --- a/llvm/test/Transforms/NewGVN/pr113997.ll +++ b/llvm/test/Transforms/NewGVN/pr113997.ll @@ -32,6 +32,8 @@ if.then: ret i1 false } +; Make sure we don't merge these two users of the incompatible call pair. + define i1 @bucket2(i32 noundef %x) { ; CHECK-LABEL: define i1 @bucket2( ; CHECK-SAME: i32 noundef [[X:%.*]]) { @@ -42,7 +44,9 @@ define i1 @bucket2(i32 noundef %x) { ; 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 [[CTPOP1INC]], 2 +; CHECK-NEXT: [[CTPOP2:%.*]] = tail call range(i32 0, 33) i32 @llvm.ctpop.i32(i32 [[X]]) +; CHECK-NEXT: [[CTPOP2INC:%.*]] = add i32 [[CTPOP2]], 1 +; CHECK-NEXT: [[RES:%.*]] = icmp eq i32 [[CTPOP2INC]], 2 ; CHECK-NEXT: ret i1 [[RES]] ; CHECK: [[IF_THEN]]: ; CHECK-NEXT: ret i1 false From 6556d1b73bdd87381dbc2fdb966721397a74242a Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Fri, 1 Nov 2024 23:47:59 +0800 Subject: [PATCH 3/4] [GVN] Remove optional. --- llvm/lib/Transforms/Scalar/GVN.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index 4dfbc73f3539a..6ef6ffef0592d 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -143,7 +143,7 @@ struct llvm::GVNPass::Expression { Type *type = nullptr; SmallVector varargs; - std::optional attrs; + AttributeList attrs; Expression(uint32_t o = ~2U) : opcode(o) {} @@ -156,10 +156,10 @@ struct llvm::GVNPass::Expression { return false; if (varargs != other.varargs) return false; - if (attrs.has_value() != other.attrs.has_value()) + if (attrs.isEmpty() != other.attrs.isEmpty()) return false; - if (attrs.has_value() && - !attrs->intersectWith(type->getContext(), *other.attrs).has_value()) + if (!attrs.isEmpty() && + !attrs.intersectWith(type->getContext(), other.attrs).has_value()) return false; return true; } From a84bc7a133d7f56047acdf4230470ce4caf675f5 Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Fri, 1 Nov 2024 23:53:45 +0800 Subject: [PATCH 4/4] [GVN] Fix empty check. --- llvm/lib/Transforms/Scalar/GVN.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/llvm/lib/Transforms/Scalar/GVN.cpp b/llvm/lib/Transforms/Scalar/GVN.cpp index 6ef6ffef0592d..13bf0e7dfc87b 100644 --- a/llvm/lib/Transforms/Scalar/GVN.cpp +++ b/llvm/lib/Transforms/Scalar/GVN.cpp @@ -156,9 +156,7 @@ struct llvm::GVNPass::Expression { return false; if (varargs != other.varargs) return false; - if (attrs.isEmpty() != other.attrs.isEmpty()) - return false; - if (!attrs.isEmpty() && + if (!attrs.isEmpty() && !other.attrs.isEmpty() && !attrs.intersectWith(type->getContext(), other.attrs).has_value()) return false; return true;