[llvm] [SandboxVec][Scheduler][DAG] Fix the update of DGNode on setOperand (PR #214110)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 4 18:17:47 PDT 2026
https://github.com/vporpo created https://github.com/llvm/llvm-project/pull/214110
The callback for updating the DAG when a Use is set was part of the DAG. But the DAG is not aware of the scheduling direction so it would try to update the UnscheduledPreds even in the bottom-up direction, which would cause an assertion failure.
The fix is to guard the Unscheduled counters based on the scheduling direction. But for this to work it has to be owned by the Scheduler. So this patch also move the callback to the scheduler.
A couple of unittests that check the Unscheduled counters also had to be moved from the DAG to the scheduler because the callback wouldn't run without a scheduler object.
To help catch similar issues in the future, this patch also implements verifiers for the DGNode and DAG with checks for UnscheduledPreds/Succs. These verifiers need to be extended in the future to cover the whole state.
>From 8d35940221043ccb684601e80a0c8d04d15bc82e Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vasileios.porpodas at amd.com>
Date: Tue, 4 Aug 2026 17:43:38 -0700
Subject: [PATCH] [SandboxVec][Scheduler][DAG] Fix the update of DGNode on
setOperand
The callback for updating the DAG when a Use is set was part of the DAG.
But the DAG is not aware of the scheduling direction so it would try to
update the UnscheduledPreds even in the bottom-up direction, which would
cause an assertion failure.
The fix is to guard the Unscheduled counters based on the scheduling direction.
But for this to work it has to be owned by the Scheduler. So this patch
also move the callback to the scheduler.
A couple of unittests that check the Unscheduled counters also had to be moved from
the DAG to the scheduler because the callback wouldn't run without a scheduler object.
To help catch similar issues in the future, this patch also implements
verifiers for the DGNode and DAG with checks for UnscheduledPreds/Succs.
These verifiers need to be extended in the future to cover the whole state.
---
.../SandboxVectorizer/DependencyGraph.h | 13 ++-
.../Vectorize/SandboxVectorizer/Scheduler.h | 8 ++
.../SandboxVectorizer/DependencyGraph.cpp | 88 +++++++++--------
.../Vectorize/SandboxVectorizer/Scheduler.cpp | 51 ++++++++++
.../scheduling_assertion_error.ll | 42 ++++++++
.../SandboxVectorizer/DependencyGraphTest.cpp | 93 ------------------
.../SandboxVectorizer/SchedulerTest.cpp | 98 +++++++++++++++++++
7 files changed, 253 insertions(+), 140 deletions(-)
create mode 100644 llvm/test/Transforms/SandboxVectorizer/scheduling_assertion_error.ll
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index 4bd972249998a..d64d68ebb0941 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -157,6 +157,10 @@ class LLVM_ABI DGNode {
DGNode(Instruction *I, DGNodeID ID) : I(I), SubclassID(ID) {}
friend class MemDGNode; // For constructor.
friend class DependencyGraph; // For UnscheduledSuccs
+#ifndef NDEBUG
+ unsigned countUnscheduledPreds(const DependencyGraph &DAG) const;
+ unsigned countUnscheduledSuccs(const DependencyGraph &DAG) const;
+#endif
public:
DGNode(Instruction *I) : I(I), SubclassID(DGNodeID::DGNode) {
@@ -302,6 +306,7 @@ class LLVM_ABI DGNode {
Instruction *getInstruction() const { return I; }
#ifndef NDEBUG
+ void verify(const DependencyGraph &DAG, bool BottomUp) const;
virtual void print(raw_ostream &OS, bool PrintDeps = true) const;
friend raw_ostream &operator<<(raw_ostream &OS, DGNode &N) {
N.print(OS);
@@ -451,7 +456,6 @@ class DependencyGraph {
std::optional<Context::CallbackID> CreateInstrCB;
std::optional<Context::CallbackID> EraseInstrCB;
std::optional<Context::CallbackID> MoveInstrCB;
- std::optional<Context::CallbackID> SetUseCB;
std::unique_ptr<BatchAAResults> BatchAA;
@@ -507,8 +511,6 @@ class DependencyGraph {
/// Called by the callbacks when instruction \p I is about to be moved to
/// \p To.
LLVM_ABI void notifyMoveInstr(Instruction *I, const BBIterator &To);
- /// Called by the callbacks when \p U's source is about to be set to \p NewSrc
- LLVM_ABI void notifySetUse(const Use &U, Value *NewSrc);
public:
/// This constructor also registers callbacks.
@@ -522,8 +524,6 @@ class DependencyGraph {
[this](Instruction *I, const BBIterator &To) {
notifyMoveInstr(I, To);
});
- SetUseCB = Ctx.registerSetUseCallback(
- [this](const Use &U, Value *NewSrc) { notifySetUse(U, NewSrc); });
}
~DependencyGraph() {
if (CreateInstrCB)
@@ -532,8 +532,6 @@ class DependencyGraph {
Ctx->unregisterEraseInstrCallback(*EraseInstrCB);
if (MoveInstrCB)
Ctx->unregisterMoveInstrCallback(*MoveInstrCB);
- if (SetUseCB)
- Ctx->unregisterSetUseCallback(*SetUseCB);
}
DGNode *getNode(Instruction *I) const {
@@ -573,6 +571,7 @@ class DependencyGraph {
"Interval and InstrToNodeMap out of sync!");
return IsEmpty;
}
+ void verify(bool BottomUp) const;
void print(raw_ostream &OS) const;
LLVM_DUMP_METHOD void dump() const;
#endif // NDEBUG
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
index e5953c1a1365c..efab3c8294bd0 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
@@ -297,11 +297,15 @@ class Scheduler {
BasicBlock *ScheduledBB = nullptr;
/// The ID of the callback we register with Sandbox IR.
std::optional<Context::CallbackID> CreateInstrCB;
+ std::optional<Context::CallbackID> SetUseCB;
/// Called by Sandbox IR's callback system, after \p I has been created.
/// NOTE: This should run after DAG's callback has run.
// TODO: Perhaps call DAG's notify function from within this one?
LLVM_ABI void notifyCreateInstr(Instruction *I);
+ /// Called by the callbacks when \p U's source is about to be set to \p NewSrc
+ LLVM_ABI void notifySetUse(const Use &U, Value *NewSrc);
+
/// \Returns a scheduling bundle containing \p Instrs.
SchedBundle *createBundle(ArrayRef<Instruction *> Instrs);
void eraseBundle(SchedBundle *SB);
@@ -342,10 +346,14 @@ class Scheduler {
// before it and updating the DAG accordingly.
CreateInstrCB = Ctx.registerCreateInstrCallback(
[this](Instruction *I) { notifyCreateInstr(I); });
+ SetUseCB = Ctx.registerSetUseCallback(
+ [this](const Use &U, Value *NewSrc) { notifySetUse(U, NewSrc); });
}
~Scheduler() {
if (CreateInstrCB)
Ctx.unregisterCreateInstrCallback(*CreateInstrCB);
+ if (SetUseCB)
+ Ctx.unregisterSetUseCallback(*SetUseCB);
}
/// Tries to build a schedule that includes all of \p Instrs scheduled at the
/// same scheduling cycle. This essentially checks that there are no
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index 1bc563fab26af..384572fec66f5 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -134,6 +134,24 @@ void DGNode::setSchedBundle(SchedBundle &SB) {
this->SB = &SB;
}
+#ifndef NDEBUG
+unsigned DGNode::countUnscheduledPreds(const DependencyGraph &DAG) const {
+ unsigned CntUnscheduledPreds = 0;
+ for (DGNode *Pred : preds(const_cast<DependencyGraph &>(DAG)))
+ if (!Pred->scheduled())
+ ++CntUnscheduledPreds;
+ return CntUnscheduledPreds;
+}
+
+unsigned DGNode::countUnscheduledSuccs(const DependencyGraph &DAG) const {
+ unsigned CntUnscheduledSuccs = 0;
+ for (DGNode *Pred : succs(const_cast<DependencyGraph &>(DAG)))
+ if (!Pred->scheduled())
+ ++CntUnscheduledSuccs;
+ return CntUnscheduledSuccs;
+}
+#endif
+
DGNode::~DGNode() {
if (SB == nullptr)
return;
@@ -141,6 +159,29 @@ DGNode::~DGNode() {
}
#ifndef NDEBUG
+void DGNode::verify(const DependencyGraph &DAG, bool BottomUp) const {
+ // TODO: Implement a complete set of checks for the DGNode state.
+ if (!Scheduled) {
+ assert(UnscheduledPreds.has_value() &&
+ "nullopt is only for scheduled nodes!");
+ assert(UnscheduledSuccs.has_value() &&
+ "nullopt is only for scheduled nodes!");
+
+ if (BottomUp) {
+ unsigned cntUnscheduledSuccs = countUnscheduledSuccs(DAG);
+ assert(*UnscheduledSuccs == cntUnscheduledSuccs &&
+ "Broken UnscheduledSuccs!");
+ } else {
+ unsigned cntUnscheduledPreds = countUnscheduledPreds(DAG);
+ assert(*UnscheduledPreds == cntUnscheduledPreds &&
+ "Broken UnscheduledPreds!");
+ }
+ } else {
+ assert(!UnscheduledPreds.has_value() && "Expected nullopt!");
+ assert(!UnscheduledSuccs.has_value() && "Expected nullopt!");
+ }
+}
+
void DGNode::print(raw_ostream &OS, bool PrintDeps) const {
OS << *I << " USuccs:" << UnscheduledSuccs << " UPreds:" << UnscheduledPreds
<< " Sched:" << Scheduled << "\n";
@@ -594,46 +635,6 @@ void DependencyGraph::notifyEraseInstr(Instruction *I) {
InstrToNodeMap.erase(I);
}
-void DependencyGraph::notifySetUse(const Use &U, Value *NewSrc) {
- // If U.User is not in the DAG, then we should not attempt to decrement
- // CurrSrcN's unscheduled successors.
- // ------- ------- -
- // CurrSrc | DAG interval
- // | NewSrc |
- // ---|--- ---|--- -
- // U.User U.User
- auto *UserI = dyn_cast_or_null<Instruction>(U.getUser());
- if (UserI == nullptr)
- return;
- auto *UserN = getNode(UserI);
- if (UserN == nullptr)
- return;
- // If UserN is marked as scheduled then we should not update CrrSrcN' or
- // NewSrcN's unscheduled successors.
- if (UserN->scheduled())
- return;
- // Update the UnscheduledSuccs counter for both the current source and
- // NewSrc if needed.
- if (auto *CurrSrcI = dyn_cast<Instruction>(U.get())) {
- if (auto *CurrSrcN = getNode(CurrSrcI)) {
- // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
- if (!CurrSrcN->scheduled()) {
- CurrSrcN->decrUnscheduledSuccs();
- UserN->decrUnscheduledPreds();
- }
- }
- }
- if (auto *NewSrcI = dyn_cast<Instruction>(NewSrc)) {
- if (auto *NewSrcN = getNode(NewSrcI)) {
- // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
- if (!NewSrcN->scheduled()) {
- NewSrcN->incrUnscheduledSuccs();
- UserN->incrUnscheduledPreds();
- }
- }
- }
-}
-
Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
if (Instrs.empty())
return {};
@@ -739,6 +740,13 @@ Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
}
#ifndef NDEBUG
+void DependencyGraph::verify(bool BottomUp) const {
+ // TODO: Implement a complete set of checks for the DAG state.
+ for (auto &[I, NPtr] : InstrToNodeMap) {
+ NPtr->verify(*this, BottomUp);
+ }
+}
+
void DependencyGraph::print(raw_ostream &OS) const {
// InstrToNodeMap is unordered so we need to create an ordered vector.
SmallVector<DGNode *> Nodes;
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
index 9a8a4099e9c50..18ca9c53f4554 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
@@ -126,6 +126,9 @@ void Scheduler::scheduleAndUpdateReadyList(SchedBundle &Bndl) {
}
}
N->setScheduled();
+#ifndef NDEBUG
+ N->verify(DAG, Dir == SchedDirection::BottomUp);
+#endif
}
}
@@ -164,6 +167,54 @@ void Scheduler::notifyCreateInstr(Instruction *I) {
}
}
+void Scheduler::notifySetUse(const Use &U, Value *NewSrc) {
+ // If U.User is not in the DAG, then we should not attempt to decrement
+ // CurrSrcN's unscheduled successors.
+ // ------- ------- -
+ // CurrSrc | DAG interval
+ // | NewSrc |
+ // ---|--- ---|--- -
+ // U.User U.User
+ auto *UserI = dyn_cast_or_null<Instruction>(U.getUser());
+ if (UserI == nullptr)
+ return;
+ auto *UserN = DAG.getNode(UserI);
+ if (UserN == nullptr)
+ return;
+ // If UserN is marked as scheduled then we should not update CrrSrcN' or
+ // NewSrcN's unscheduled successors.
+ if (UserN->scheduled())
+ return;
+ // Update the UnscheduledSuccs counter for both the current source and
+ // NewSrc if needed.
+ if (auto *CurrSrcI = dyn_cast<Instruction>(U.get())) {
+ if (auto *CurrSrcN = DAG.getNode(CurrSrcI)) {
+ // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
+ if (!CurrSrcN->scheduled()) {
+ if (Dir == SchedDirection::BottomUp) {
+ CurrSrcN->decrUnscheduledSuccs();
+ } else {
+ assert(Dir == SchedDirection::TopDown);
+ UserN->decrUnscheduledPreds();
+ }
+ }
+ }
+ }
+ if (auto *NewSrcI = dyn_cast<Instruction>(NewSrc)) {
+ if (auto *NewSrcN = DAG.getNode(NewSrcI)) {
+ // If CurrSrcN is scheduled there is no point in updating UnscheduleSuccs.
+ if (!NewSrcN->scheduled()) {
+ if (Dir == SchedDirection::BottomUp) {
+ NewSrcN->incrUnscheduledSuccs();
+ } else {
+ assert(Dir == SchedDirection::TopDown);
+ UserN->incrUnscheduledPreds();
+ }
+ }
+ }
+ }
+}
+
SchedBundle *Scheduler::createBundle(ArrayRef<Instruction *> Instrs) {
SchedBundle::ContainerTy Nodes;
Nodes.reserve(Instrs.size());
diff --git a/llvm/test/Transforms/SandboxVectorizer/scheduling_assertion_error.ll b/llvm/test/Transforms/SandboxVectorizer/scheduling_assertion_error.ll
new file mode 100644
index 0000000000000..6c5e57b98bf88
--- /dev/null
+++ b/llvm/test/Transforms/SandboxVectorizer/scheduling_assertion_error.ll
@@ -0,0 +1,42 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6
+; RUN: opt -passes=sandbox-vectorizer -sbvec-passes='regions-from-metadata<bundle-vec(bottom-up)>' -S %s | FileCheck %s
+; REQUIRES: asserts
+
+; This checks that this IR does not trigger assertion errors.
+; The SetUseCB notifier's logic was not aware of the scheduling direction
+; and would update the UnscheduledPreds/Succs counters for the non-active
+; scheduling direction, which would trigger assertion errors.
+define void @foo(ptr %ptr) {
+; CHECK-LABEL: define void @foo(
+; CHECK-SAME: ptr [[PTR:%.*]]) {
+; CHECK-NEXT: [[GEP0:%.*]] = getelementptr i64, ptr [[PTR]], i64 0
+; CHECK-NEXT: [[VECL:%.*]] = load <2 x i64>, ptr [[GEP0]], align 4, !sandboxvec [[META0:![0-9]+]]
+; CHECK-NEXT: [[UNPACK:%.*]] = extractelement <2 x i64> [[VECL]], i32 0, !sandboxvec [[META0]]
+; CHECK-NEXT: [[SUB:%.*]] = sub i64 [[UNPACK]], 0
+; CHECK-NEXT: [[PACK:%.*]] = insertelement <2 x i64> poison, i64 [[SUB]], i32 0, !sandboxvec [[META0]]
+; CHECK-NEXT: [[PACK1:%.*]] = insertelement <2 x i64> [[PACK]], i64 0, i32 1, !sandboxvec [[META0]]
+; CHECK-NEXT: [[VEC:%.*]] = and <2 x i64> [[PACK1]], zeroinitializer, !sandboxvec [[META0]]
+; CHECK-NEXT: [[VEC2:%.*]] = or <2 x i64> [[VECL]], [[VEC]], !sandboxvec [[META0]]
+; CHECK-NEXT: store <2 x i64> [[VEC2]], ptr [[GEP0]], align 8, !sandboxvec [[META0]]
+; CHECK-NEXT: ret void
+;
+ %gep0 = getelementptr i64, ptr %ptr, i64 0
+ %gep1 = getelementptr i64, ptr %ptr, i64 1
+ %ld0 = load i64, ptr %gep0
+ %ld1 = load i64, ptr %gep1
+ %sub = sub i64 %ld0, 0
+ %andB0 = and i64 %sub, 0
+ %andB1 = and i64 0, 0
+ %or0 = or i64 %ld0, %andB0
+ %or1 = or i64 %ld1, %andB1
+ store i64 %or0, ptr %gep0, align 8, !sandboxaux !1, !sandboxvec !0
+ store i64 %or1, ptr %gep1, align 8, !sandboxaux !2, !sandboxvec !0
+ ret void
+}
+
+!0 = distinct !{!"sandboxregion"}
+!1 = !{i32 0}
+!2 = !{i32 1}
+;.
+; CHECK: [[META0]] = distinct !{!"sandboxregion"}
+;.
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
index b789b421d1563..44b323f41563c 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
@@ -1235,69 +1235,6 @@ define void @foo(ptr %ptr, i8 %v, i8 %v0, i8 %v1, i8 %v2, i8 %v3) {
EXPECT_TRUE(memDependency(DAG.getNode(S0), DAG.getNode(S1)));
}
-// Setting a Use with a setOperand(), RUW, RAUW etc. can add/remove use-def
-// edges. This needs to maintain the UnscheduledSuccs and UnscheduledPreds
-// counters.
-TEST_F(DependencyGraphTest, MaintainUnscheduledSuccsOnUseSet) {
- parseIR(C, R"IR(
-define void @foo(i8 %v0, i8 %v1) {
- %add0 = add i8 %v0, %v1
- %add1 = add i8 %add0, %v1
- ret void
-}
-)IR");
- llvm::Function *LLVMF = &*M->getFunction("foo");
- sandboxir::Context Ctx(C);
- auto *F = Ctx.createFunction(LLVMF);
- auto *Arg0 = F->getArg(0);
- auto *BB = &*F->begin();
- auto It = BB->begin();
- auto *Add0 = cast<sandboxir::BinaryOperator>(&*It++);
- auto *Add1 = cast<sandboxir::BinaryOperator>(&*It++);
- sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
- DAG.extend({Add0, Add1});
- auto *N0 = DAG.getNode(Add0);
- auto *N1 = DAG.getNode(Add1);
-
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
- // Now change %add1 operand to not use %add0.
- Add1->setOperand(0, Arg0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
-
- // RAUW
- Add0->replaceAllUsesWith(Arg0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
-
- // RUWIf
- Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
-
- // RUOW
- Add1->replaceUsesOfWith(Add0, Arg0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
- EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
-}
-
// Make sure we maintain the unscheduled succs when the use-def edges cross the
// DAG boundaries, i.e., when have an external user.
TEST_F(DependencyGraphTest, MaintainUnscheduledSuccsExtUser) {
@@ -1487,36 +1424,6 @@ define void @foo(i8 %v0) {
EXPECT_EQ(Add1N->getNumUnscheduledSuccs(), 0u);
}
-// Don't udpate the unscheduled preds if the operand is scheduled.
-TEST_F(DependencyGraphTest, MaintainUnscheduledPredsWhenOperandScheduled) {
- parseIR(C, R"IR(
-define void @foo(i8 %v0) {
- %sched = add i8 %add0, 1
- %add0 = add i8 %v0, 0
- ret void
-}
-)IR");
- llvm::Function *LLVMF = &*M->getFunction("foo");
- sandboxir::Context Ctx(C);
- auto *F = Ctx.createFunction(LLVMF);
- auto *BB = &*F->begin();
- auto It = BB->begin();
- auto *Sched = cast<sandboxir::BinaryOperator>(&*It++);
- auto *Add0 = cast<sandboxir::BinaryOperator>(&*It++);
- sandboxir::DependencyGraph DAG(getAA(*LLVMF), Ctx);
- DAG.extend({Sched, Add0});
- auto *SchedN = DAG.getNode(Sched);
- auto *Add0N = DAG.getNode(Add0);
- EXPECT_EQ(Add0N->getNumUnscheduledPreds(), 0u);
- // Mark SchedN as scheduled
- SchedN->setScheduled();
-
- // Change Add0's operand and make sure that this won't update Add0N's
- // unscheduled preds because SchedN is "scheduled".
- Add0->setOperand(0, Sched);
- EXPECT_EQ(Add0N->getNumUnscheduledPreds(), 0u);
-}
-
// When erasing a non-mem instruction we must not touch the UnscheduledSuccs
// of an already-scheduled predecessor, since that counter is set to
// std::nullopt once a node is scheduled.
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
index 250cf63e76675..308d3d3b4ebb4 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
@@ -1109,3 +1109,101 @@ define void @foo(ptr %ptr, i8 %v0) {
EXPECT_DEATH(BeforeBegin.getIterator(), ".*Expected.*");
#endif
}
+
+
+// Don't udpate the unscheduled preds if the operand is scheduled.
+TEST_F(SchedulerTest, MaintainUnscheduledPredsWhenOperandScheduled) {
+ parseIR(C, R"IR(
+define void @foo(i8 %v0) {
+ %sched = add i8 %add0, 1
+ %add0 = add i8 %v0, 0
+ ret void
+}
+)IR");
+ llvm::Function *LLVMF = &*M->getFunction("foo");
+ sandboxir::Context Ctx(C);
+ auto *F = Ctx.createFunction(LLVMF);
+ auto *BB = &*F->begin();
+ auto It = BB->begin();
+ auto *Sched = cast<sandboxir::BinaryOperator>(&*It++);
+ auto *Add0 = cast<sandboxir::BinaryOperator>(&*It++);
+ sandboxir::Scheduler Scheduler(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
+ auto &DAG = sandboxir::SchedulerInternalsAttorney::getDAG(Scheduler);
+ DAG.extend({Sched, Add0});
+ auto *SchedN = DAG.getNode(Sched);
+ auto *Add0N = DAG.getNode(Add0);
+ EXPECT_EQ(Add0N->getNumUnscheduledPreds(), 0u);
+ // Mark SchedN as scheduled
+ SchedN->setScheduled();
+
+ // Change Add0's operand and make sure that this won't update Add0N's
+ // unscheduled preds because SchedN is "scheduled".
+ Add0->setOperand(0, Sched);
+ EXPECT_EQ(Add0N->getNumUnscheduledPreds(), 0u);
+}
+
+// Setting a Use with a setOperand(), RUW, RAUW etc. can add/remove use-def
+// edges. This needs to maintain the UnscheduledSuccs and UnscheduledPreds
+// counters.
+TEST_F(SchedulerTest, MaintainUnscheduledSuccsOnUseSet) {
+ parseIR(C, R"IR(
+define void @foo(i8 %v0, i8 %v1) {
+ %add0 = add i8 %v0, %v1
+ %add1 = add i8 %add0, %v1
+ ret void
+}
+)IR");
+ llvm::Function *LLVMF = &*M->getFunction("foo");
+ sandboxir::Context Ctx(C);
+ auto *F = Ctx.createFunction(LLVMF);
+ auto *Arg0 = F->getArg(0);
+ auto *BB = &*F->begin();
+ auto It = BB->begin();
+ auto *Add0 = cast<sandboxir::BinaryOperator>(&*It++);
+ auto *Add1 = cast<sandboxir::BinaryOperator>(&*It++);
+ sandboxir::Scheduler Scheduler(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
+ auto &DAG = sandboxir::SchedulerInternalsAttorney::getDAG(Scheduler);
+ DAG.extend({Add0, Add1});
+ auto *N0 = DAG.getNode(Add0);
+ auto *N1 = DAG.getNode(Add1);
+
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
+ // Now change %add1 operand to not use %add0.
+ Add1->setOperand(0, Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
+
+ // RAUW
+ Add0->replaceAllUsesWith(Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
+
+ // RUWIf
+ Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
+
+ // RUOW
+ Add1->replaceUsesOfWith(Add0, Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 0u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledSuccs(), 1u);
+ EXPECT_EQ(N1->getNumUnscheduledPreds(), 1u);
+}
More information about the llvm-commits
mailing list