[llvm] [SandboxVec][Scheduler] Add direction arg to Scheduler constructor (PR #208602)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 9 18:48:13 PDT 2026
https://github.com/vporpo created https://github.com/llvm/llvm-project/pull/208602
With this patch we require the user to set the scheduling direction during construction. The direction used to default to BottomUp which would cause crashes if the user had forgotten to set the direction and attempt to scheduler top-down.
Also drop Scheduler::setDirection() as there is no longer a need for it.
>From 86a947aac002f3f1edbede2c3f6180dcdddcf2b2 Mon Sep 17 00:00:00 2001
From: Vasileios Porpodas <vasileios.porpodas at amd.com>
Date: Thu, 9 Jul 2026 18:40:06 -0700
Subject: [PATCH] [SandboxVec][Scheduler] Add direction arg to Scheduler
constructor
With this patch we require the user to set the scheduling direction during
construction. The direction used to default to BottomUp which would cause
crashes if the user had forgotten to set the direction and attempt to
scheduler top-down.
Also drop Scheduler::setDirection() as there is no longer a need for it.
---
.../Vectorize/SandboxVectorizer/Legality.h | 4 +-
.../Vectorize/SandboxVectorizer/Scheduler.h | 9 +--
.../SandboxVectorizer/Passes/BottomUpVec.cpp | 2 +-
.../SandboxVectorizer/Passes/LoadStoreVec.cpp | 2 +-
.../SandboxVectorizer/LegalityTest.cpp | 9 ++-
.../SandboxVectorizer/SchedulerTest.cpp | 66 +++++++++++--------
6 files changed, 52 insertions(+), 40 deletions(-)
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
index b1fa6ed7742e8..90a74f58e85ef 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Legality.h
@@ -335,8 +335,8 @@ class LegalityAnalysis {
public:
LegalityAnalysis(AAResults &AA, ScalarEvolution &SE, const DataLayout &DL,
- Context &Ctx, InstrMaps &IMaps)
- : Sched(AA, Ctx), SE(SE), DL(DL), IMaps(IMaps) {}
+ Context &Ctx, InstrMaps &IMaps, SchedDirection Dir)
+ : Sched(AA, Ctx, Dir), SE(SE), DL(DL), IMaps(IMaps) {}
/// A LegalityResult factory.
template <typename ResultT, typename... ArgsT>
ResultT &createLegalityResult(ArgsT &&...Args) {
diff --git a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
index 86ab5fb8b464b..f406560b246a6 100644
--- a/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
+++ b/llvm/include/llvm/Transforms/Vectorize/SandboxVectorizer/Scheduler.h
@@ -230,7 +230,8 @@ class Scheduler {
SchedDirection Dir = SchedDirection::BottomUp;
public:
- Scheduler(AAResults &AA, Context &Ctx) : DAG(AA, Ctx), Ctx(Ctx) {
+ Scheduler(AAResults &AA, Context &Ctx, SchedDirection Dir)
+ : DAG(AA, Ctx), Ctx(Ctx), Dir(Dir) {
// NOTE: The scheduler's callback depends on the DAG's callback running
// before it and updating the DAG accordingly.
CreateInstrCB = Ctx.registerCreateInstrCallback(
@@ -240,12 +241,6 @@ class Scheduler {
if (CreateInstrCB)
Ctx.unregisterCreateInstrCallback(*CreateInstrCB);
}
- void setDirection(SchedDirection NewDir) {
- assert(Bndls.empty() && DAG.empty() && ReadyList.empty() &&
- !ScheduleTopItOpt && ScheduledBB == nullptr &&
- "We can't change the direction during scheduling!");
- Dir = NewDir;
- }
/// Tries to build a schedule that includes all of \p Instrs scheduled at the
/// same scheduling cycle. This essentially checks that there are no
/// dependencies among \p Instrs. This function may involve scheduling
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
index 6bf257fcf8b1d..d0f8b7ad12e34 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/BottomUpVec.cpp
@@ -540,7 +540,7 @@ bool BottomUpVec::runOnRegion(Region &Rgn, const Analyses &A) {
IMaps = std::make_unique<InstrMaps>();
LegalityAnalysis Legality(A.getAA(), A.getScalarEvolution(),
F.getParent()->getDataLayout(), F.getContext(),
- *IMaps);
+ *IMaps, SchedDirection::BottomUp);
// TODO: Refactor to remove the unnecessary copy to SeedSliceVals.
SmallVector<Value *> SeedSliceVals(SeedSlice.begin(), SeedSlice.end());
diff --git a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
index 1ce435cecdf5e..adf9ee32a3533 100644
--- a/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
+++ b/llvm/lib/Transforms/Vectorize/SandboxVectorizer/Passes/LoadStoreVec.cpp
@@ -71,7 +71,7 @@ bool LoadStoreVec::runOnRegion(Region &Rgn, const Analyses &A) {
Function &F = *Bndl[0]->getParent()->getParent();
DL = &F.getParent()->getDataLayout();
auto &Ctx = F.getContext();
- Scheduler Sched(A.getAA(), Ctx);
+ Scheduler Sched(A.getAA(), Ctx, SchedDirection::BottomUp);
if (!VecUtils::areConsecutive<StoreInst, Instruction>(
Bndl, A.getScalarEvolution(), *DL))
return false;
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
index 586f846caf267..e5ddf53b598c1 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/LegalityTest.cpp
@@ -137,7 +137,8 @@ define void @foo(ptr %ptr, <2 x float> %vec2, <3 x float> %vec3, i8 %arg, float
auto *Sel1 = cast<sandboxir::SelectInst>(&*It++);
llvm::sandboxir::InstrMaps IMaps;
- sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps);
+ sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps,
+ sandboxir::SchedDirection::BottomUp);
const auto &Result =
Legality.canVectorize({St0, St1}, /*SkipScheduling=*/true);
EXPECT_TRUE(isa<sandboxir::Widen>(Result));
@@ -289,7 +290,8 @@ define void @foo(ptr %ptr) {
auto *St1 = cast<sandboxir::StoreInst>(&*It++);
llvm::sandboxir::InstrMaps IMaps;
- sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps);
+ sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps,
+ sandboxir::SchedDirection::BottomUp);
{
// Can vectorize St0,St1.
const auto &Result = Legality.canVectorize({St0, St1});
@@ -325,7 +327,8 @@ define void @foo() {
sandboxir::Context Ctx(C);
llvm::sandboxir::InstrMaps IMaps;
- sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps);
+ sandboxir::LegalityAnalysis Legality(*AA, *SE, DL, Ctx, IMaps,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(
Matches(Legality.createLegalityResult<sandboxir::Widen>(), "Widen"));
EXPECT_TRUE(Matches(Legality.createLegalityResult<sandboxir::Pack>(
diff --git a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
index 0c756e6b705a0..7f7f303a90f51 100644
--- a/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/SandboxVectorizer/SchedulerTest.cpp
@@ -241,20 +241,23 @@ define void @foo(ptr %ptr, i8 %v0, i8 %v1) {
{
// Schedule all instructions in sequence.
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S1}));
EXPECT_TRUE(Sched.trySchedule({S0}));
}
{
// Skip instructions.
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S0}));
}
{
// Try invalid scheduling. Dependency S0->S1.
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_FALSE(Sched.trySchedule({S0, S1}));
}
@@ -285,8 +288,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({Ret}));
@@ -294,8 +297,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
}
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
EXPECT_TRUE(Sched.trySchedule({S1}));
EXPECT_TRUE(Sched.trySchedule({S0}));
@@ -304,8 +307,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
}
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
EXPECT_TRUE(Sched.trySchedule({S0}));
EXPECT_TRUE(Sched.trySchedule({S1}));
@@ -315,8 +318,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
}
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
EXPECT_TRUE(Sched.trySchedule({L0}));
EXPECT_TRUE(Sched.trySchedule({L1}));
EXPECT_FALSE(Sched.trySchedule({S0, S2}));
@@ -326,8 +329,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
}
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
EXPECT_TRUE(Sched.trySchedule({L1}));
EXPECT_TRUE(Sched.trySchedule({L0}));
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
@@ -336,8 +339,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
}
{
Ctx.save();
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
- Sched.setDirection(sandboxir::SchedDirection::TopDown);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::TopDown);
// Dependent instrs.
EXPECT_FALSE(Sched.trySchedule({L0, L1, S0, S1}));
EXPECT_FALSE(Sched.trySchedule({L0, L1, S0}));
@@ -374,7 +377,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1) {
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
@@ -407,7 +411,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, i8 %arg) {
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
@@ -457,7 +462,8 @@ define void @foo(ptr %ptr, i16 %arg) {
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({Zext0, Zext1}));
EXPECT_TRUE(Sched.trySchedule({Shl0, Shl1}));
@@ -496,7 +502,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, ptr noalias %ptr2) {
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({Add0, Add1}));
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
@@ -533,7 +540,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, ptr noalias %ptr2) {
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({Add0, Add1}));
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
@@ -570,7 +578,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, ptr noalias %ptr2) {
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
EXPECT_TRUE(Sched.trySchedule({Add0, Add1}));
EXPECT_TRUE(Sched.trySchedule({L0, L1}));
@@ -609,7 +618,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, ptr noalias %ptr2) {
auto *S1 = cast<sandboxir::StoreInst>(&*It++);
auto *S0 = cast<sandboxir::StoreInst>(&*It++);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
auto &DAG = sandboxir::SchedulerInternalsAttorney::getDAG(Sched);
auto GetBndlSchedState = [&Sched](ArrayRef<sandboxir::Instruction *> Instrs) {
return sandboxir::SchedulerInternalsAttorney::getBndlSchedState(Sched,
@@ -729,7 +739,8 @@ define void @foo(ptr noalias %ptrA0, ptr noalias %ptrA1,
auto *Ret = cast<sandboxir::ReturnInst>(&*It++);
(void)Ret;
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({A0, A1}));
// NOTE: We schedule the intermediate nodes between {A0,A1} and {B0,B1} by
// hand one by one to make sure they are scheduled in that order because
@@ -781,7 +792,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, i8 %v0, i8 %v1) {
{
// Schedule bottom-up
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S0, S1}));
// Scheduling across blocks should fail.
@@ -789,7 +801,8 @@ define void @foo(ptr noalias %ptr0, ptr noalias %ptr1, i8 %v0, i8 %v1) {
}
{
// Schedule top-down
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
EXPECT_TRUE(Sched.trySchedule({Add0, Add1}));
// Scheduling across blocks should fail.
EXPECT_FALSE(Sched.trySchedule({S0, S1}));
@@ -815,7 +828,8 @@ define void @foo(ptr noalias %ptr, ptr noalias %ptr1, ptr noalias %ptr2) {
auto *Ptr1 = F->getArg(1);
auto *Ptr2 = F->getArg(2);
- sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx);
+ sandboxir::Scheduler Sched(getAA(*LLVMF), Ctx,
+ sandboxir::SchedDirection::BottomUp);
// Schedule Ret and S0. The top of schedule should be at S0.
EXPECT_TRUE(Sched.trySchedule({Ret}));
EXPECT_TRUE(Sched.trySchedule({S0}));
More information about the llvm-commits
mailing list