[llvm] [SandboxVec][DAG][Scheduler][NFC] Move UnschedDeps maintenance to Scheduler (PR #223822)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 15:54:10 PDT 2026
https://github.com/vporpo updated https://github.com/llvm/llvm-project/pull/223822
>From 916728dd87e2dd05ac3fdf6b68c5393d001c68c1 Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vasileios.porpodas at amd.com>
Date: Wed, 9 Sep 2026 21:52:08 +0000
Subject: [PATCH 1/2] [SandboxVec][DAG][Scheduler][NFC] Move UnschedDeps
maintenance to Scheduler
The UnscheduledDeps field of the DAG's DGNode is used by the Scheduler.
So it makes sense to have the callbacks that update it in the Scheduler
and not in DependencyGraph.
---
.../SandboxVectorizer/DependencyGraph.h | 7 --
.../SandboxVectorizer/DependencyGraph.cpp | 46 --------
.../Vectorize/SandboxVectorizer/Scheduler.cpp | 57 ++++++++-
.../SandboxVectorizer/DependencyGraphTest.cpp | 104 -----------------
.../SandboxVectorizer/SchedulerTest.cpp | 108 ++++++++++++++++++
5 files changed, 163 insertions(+), 159 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
index 8f39c4258bb61..23b72cb518569 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.h
@@ -454,7 +454,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;
@@ -510,8 +509,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.
@@ -525,8 +522,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)
@@ -535,8 +530,6 @@ class DependencyGraph {
Ctx->unregisterEraseInstrCallback(*EraseInstrCB);
if (MoveInstrCB)
Ctx->unregisterMoveInstrCallback(*MoveInstrCB);
- if (SetUseCB)
- Ctx->unregisterSetUseCallback(*SetUseCB);
}
DGNode *getNode(Instruction *I) const {
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
index 1ac84df828075..746ee0b53d2a8 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/DependencyGraph.cpp
@@ -604,52 +604,6 @@ void DependencyGraph::notifyEraseInstr(Instruction *I) {
InstrToNodeMap.erase(I);
}
-void DependencyGraph::notifySetUse(const Use &U, Value *NewSrc) {
- // TODO: We should eventually move the UnschedDep logic to the scheduler.
-
- // 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 UnscheduledDeps.
- if (!CurrSrcN->scheduled()) {
- if (Dir == SchedDirection::BottomUp)
- CurrSrcN->decrUnscheduledDeps();
- else
- UserN->decrUnscheduledDeps();
- }
- }
- }
- if (auto *NewSrcI = dyn_cast<Instruction>(NewSrc)) {
- if (auto *NewSrcN = getNode(NewSrcI)) {
- // If CurrSrcN is scheduled there is no point in updating UnscheduleDeps.
- if (!NewSrcN->scheduled()) {
- if (Dir == SchedDirection::BottomUp)
- NewSrcN->incrUnscheduledDeps();
- else
- UserN->incrUnscheduledDeps();
- }
- }
- }
-}
-
Interval<Instruction> DependencyGraph::extend(ArrayRef<Instruction *> Instrs) {
if (Instrs.empty())
return {};
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
index ff79bb69f7505..2702024fa21ba 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
@@ -191,10 +191,63 @@ void Scheduler::notifyMoveInstr(Instruction *I, const BBIterator &To) {
// If this assumption does not hold, we would need to empty the ready list and
// re-fill it.
}
+
void Scheduler::notifySetUse(const Use &U, Value *NewSrc) {
// We don't maintain the state while reverting.
if (Ctx.getTracker().getState() == Tracker::TrackerState::Reverting)
return;
+
+ auto TryUpdateUnschedDeps = [this, &U, 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;
+ // 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
+ // UnscheduledDeps.
+ if (!CurrSrcN->scheduled()) {
+ if (Dir == SchedDirection::BottomUp)
+ CurrSrcN->decrUnscheduledDeps();
+ else
+ UserN->decrUnscheduledDeps();
+ }
+ }
+ }
+ if (auto *NewSrcI = dyn_cast<Instruction>(NewSrc)) {
+ if (auto *NewSrcN = DAG.getNode(NewSrcI)) {
+ // If CurrSrcN is scheduled there is no point in updating
+ // UnscheduleDeps.
+ if (!NewSrcN->scheduled()) {
+ if (Dir == SchedDirection::BottomUp)
+ NewSrcN->incrUnscheduledDeps();
+ else
+ UserN->incrUnscheduledDeps();
+ }
+ }
+ }
+ };
+ TryUpdateUnschedDeps();
+
Instruction *DstI = cast<Instruction>(U.getUser());
DGNode *DstN = DAG.getNode(DstI);
Value *OldSrc = U.get();
@@ -207,7 +260,7 @@ void Scheduler::notifySetUse(const Use &U, Value *NewSrc) {
switch (Dir) {
case SchedDirection::BottomUp: {
// Check if OldSrc is now ready and add it to the ready list.
- if (OldSrcN && OldSrcN->ready() && !OldSrcN->scheduled() &&
+ if (OldSrcN && OldSrcN->ready() && OldSrcN && !OldSrcN->scheduled() &&
!ReadyList.contains(OldSrcN))
ReadyList.insert(OldSrcN);
// Check if NewSrcN needs to be removed from the ready list.
@@ -217,7 +270,7 @@ void Scheduler::notifySetUse(const Use &U, Value *NewSrc) {
}
case SchedDirection::TopDown: {
// Check if we need to add DstN to the ready list.
- if (DstN && DstN->ready() && !NewSrcN->scheduled() &&
+ if (DstN && DstN->ready() && NewSrcN && !NewSrcN->scheduled() &&
!ReadyList.contains(NewSrcN))
ReadyList.insert(NewSrcN);
// Check if we need to remove DstN from the ready list.
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
index 299fa4265f30c..cc9321d7d6026 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/DependencyGraphTest.cpp
@@ -1296,110 +1296,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 UnscheduledSuccs.
-TEST_F(DependencyGraphTest, MaintainUnscheduledSuccsOnUseSet_BottomUp) {
- 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(BottomUp, getAA(*LLVMF), Ctx);
- DAG.extend({Add0, Add1});
- auto *N0 = DAG.getNode(Add0);
-
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
- // Now change %add1 operand to not use %add0.
- Add1->setOperand(0, Arg0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
-
- // RAUW
- Add0->replaceAllUsesWith(Arg0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
-
- // RUWIf
- Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
-
- // RUOW
- Add1->replaceUsesOfWith(Add0, Arg0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
-}
-
-// Setting a Use with a setOperand(), RUW, RAUW etc. can add/remove use-def
-// edges. This needs to maintain UnscheduledPreds.
-TEST_F(DependencyGraphTest, MaintainUnscheduledSuccsOnUseSet_TopDown) {
- 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(TopDown, getAA(*LLVMF), Ctx);
- DAG.extend({Add0, Add1});
- auto *N1 = DAG.getNode(Add1);
-
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
- // Now change %add1 operand to not use %add0.
- Add1->setOperand(0, Arg0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
-
- // RAUW
- Add0->replaceAllUsesWith(Arg0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
-
- // RUWIf
- Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
-
- // RUOW
- Add1->replaceUsesOfWith(Add0, Arg0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
- // Restore it: %add0 is now used by %add1.
- Add1->setOperand(0, Add0);
- EXPECT_EQ(N1->getNumUnscheduledDeps(), 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) {
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
index 70ffaf49b4300..371ecaa444413 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
@@ -1012,6 +1012,114 @@ define void @foo(i8 %v0) {
#endif
}
+// Setting a Use with a setOperand(), RUW, RAUW etc. can add/remove use-def
+// edges. This needs to maintain UnscheduledSuccs.
+TEST_F(SchedulerTest, MaintainUnscheduledSuccsOnUseSet_BottomUp) {
+ 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 Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
+ auto &DAG = sandboxir::SchedulerInternalsAttorney::getDAG(Sched);
+ DAG.extend({Add0, Add1});
+ auto *N0 = DAG.getNode(Add0);
+
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
+ // Now change %add1 operand to not use %add0.
+ Add1->setOperand(0, Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
+
+ // RAUW
+ Add0->replaceAllUsesWith(Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
+
+ // RUWIf
+ Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
+
+ // RUOW
+ Add1->replaceUsesOfWith(Add0, Arg0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N0->getNumUnscheduledDeps(), 1u);
+}
+
+// Setting a Use with a setOperand(), RUW, RAUW etc. can add/remove use-def
+// edges. This needs to maintain UnscheduledPreds.
+TEST_F(SchedulerTest, MaintainUnscheduledSuccsOnUseSet_TopDown) {
+ 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 Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
+ auto &DAG = sandboxir::SchedulerInternalsAttorney::getDAG(Sched);
+ DAG.extend({Add0, Add1});
+ auto *N1 = DAG.getNode(Add1);
+
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
+ // Now change %add1 operand to not use %add0.
+ Add1->setOperand(0, Arg0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
+
+ // RAUW
+ Add0->replaceAllUsesWith(Arg0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
+
+ // RUWIf
+ Add0->replaceUsesWithIf(Arg0, [](const auto &U) { return true; });
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
+
+ // RUOW
+ Add1->replaceUsesOfWith(Add0, Arg0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 0u);
+ // Restore it: %add0 is now used by %add1.
+ Add1->setOperand(0, Add0);
+ EXPECT_EQ(N1->getNumUnscheduledDeps(), 1u);
+}
+
TEST_F(SchedulerTest, NotifySetUse) {
parseIR(C, R"IR(
define void @foo(i8 %v0, i8 %v1) {
>From 96c465314915af088e9782d2c4c832471722dd6a Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vasileios.porpodas at amd.com>
Date: Tue, 15 Sep 2026 21:18:10 +0000
Subject: [PATCH 2/2] fixup! [SandboxVec][DAG][Scheduler][NFC] Move UnschedDeps
maintenance to Scheduler
---
llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
index 2702024fa21ba..a673de8de7253 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Scheduler.cpp
@@ -260,7 +260,7 @@ void Scheduler::notifySetUse(const Use &U, Value *NewSrc) {
switch (Dir) {
case SchedDirection::BottomUp: {
// Check if OldSrc is now ready and add it to the ready list.
- if (OldSrcN && OldSrcN->ready() && OldSrcN && !OldSrcN->scheduled() &&
+ if (OldSrcN && OldSrcN->ready() && !OldSrcN->scheduled() &&
!ReadyList.contains(OldSrcN))
ReadyList.insert(OldSrcN);
// Check if NewSrcN needs to be removed from the ready list.
More information about the llvm-commits
mailing list