Skip to content

[MISched] Unify the way to specify scheduling direction #119518

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions llvm/include/llvm/CodeGen/MachineScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,16 @@

namespace llvm {

extern cl::opt<bool> ForceTopDown;
extern cl::opt<bool> ForceBottomUp;
namespace MISched {
enum Direction {
Unspecified,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use getNumOccurrences()?

Copy link
Contributor Author

@wangpc-pp wangpc-pp Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is that there will be a lot of getNumOccurrences(), which makes the conditions complicated. For example:

static ScheduleDAGInstrs *createInstructionShuffler(MachineSchedContext *C) {
  bool Alternate =
      PreRADirection != MISched::TopDown && PreRADirection != MISched::BottomUp;
  bool TopDown = PreRADirection != MISched::BottomUp;
  return new ScheduleDAGMILive(
      C, std::make_unique<InstructionShuffler>(Alternate, TopDown));
}

will be:

static ScheduleDAGInstrs *createInstructionShuffler(MachineSchedContext *C) {
  bool Alternate = PreRADirection.getNumOccurrences() > 0 &&
      PreRADirection != MISched::TopDown && PreRADirection != MISched::BottomUp;
  bool TopDown = PreRADirection.getNumOccurrences() > 0 && PreRADirection != MISched::BottomUp;
  return new ScheduleDAGMILive(
      C, std::make_unique<InstructionShuffler>(Alternate, TopDown));
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I see, that makes sense. I don't have a strong opinion but would be temped to go that route and make it more explicit. The shuffler, whilst not the most important, seems to be assuming the default is Bidirectional where as the scheduler defaults to BottomUp. I have the feeling that MISched::Direction could get used more in the future, and if it does having an Unspecified makes it more awkward to rely upon.

Either way this LGTM. It's a nice cleanup.

TopDown,
BottomUp,
Bidirectional,
};
} // namespace MISched

extern cl::opt<MISched::Direction> PreRADirection;
extern cl::opt<bool> VerifyScheduling;
#ifndef NDEBUG
extern cl::opt<bool> ViewMISchedDAGs;
Expand Down
85 changes: 39 additions & 46 deletions llvm/lib/CodeGen/MachineScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,30 +77,30 @@ STATISTIC(NumClustered, "Number of load/store pairs clustered");

namespace llvm {

cl::opt<bool> ForceTopDown("misched-topdown", cl::Hidden,
cl::desc("Force top-down list scheduling"));
cl::opt<bool> ForceBottomUp("misched-bottomup", cl::Hidden,
cl::desc("Force bottom-up list scheduling"));
namespace MISchedPostRASched {
enum Direction {
TopDown,
BottomUp,
Bidirectional,
};
} // end namespace MISchedPostRASched
cl::opt<MISchedPostRASched::Direction> PostRADirection(
cl::opt<MISched::Direction> PreRADirection(
"misched-prera-direction", cl::Hidden,
cl::desc("Pre reg-alloc list scheduling direction"),
cl::init(MISched::Unspecified),
cl::values(
clEnumValN(MISched::TopDown, "topdown",
"Force top-down pre reg-alloc list scheduling"),
clEnumValN(MISched::BottomUp, "bottomup",
"Force bottom-up pre reg-alloc list scheduling"),
clEnumValN(MISched::Bidirectional, "bidirectional",
"Force bidirectional pre reg-alloc list scheduling")));

cl::opt<MISched::Direction> PostRADirection(
"misched-postra-direction", cl::Hidden,
cl::desc("Post reg-alloc list scheduling direction"),
// Default to top-down because it was implemented first and existing targets
// expect that behavior by default.
cl::init(MISchedPostRASched::TopDown),
cl::init(MISched::Unspecified),
cl::values(
clEnumValN(MISchedPostRASched::TopDown, "topdown",
clEnumValN(MISched::TopDown, "topdown",
"Force top-down post reg-alloc list scheduling"),
clEnumValN(MISchedPostRASched::BottomUp, "bottomup",
clEnumValN(MISched::BottomUp, "bottomup",
"Force bottom-up post reg-alloc list scheduling"),
clEnumValN(MISchedPostRASched::Bidirectional, "bidirectional",
clEnumValN(MISched::Bidirectional, "bidirectional",
"Force bidirectional post reg-alloc list scheduling")));

cl::opt<bool>
DumpCriticalPathLength("misched-dcpl", cl::Hidden,
cl::desc("Print critical path length to stdout"));
Expand Down Expand Up @@ -3307,19 +3307,15 @@ void GenericScheduler::initPolicy(MachineBasicBlock::iterator Begin,
RegionPolicy.ShouldTrackLaneMasks = false;
}

// Check -misched-topdown/bottomup can force or unforce scheduling direction.
// e.g. -misched-bottomup=false allows scheduling in both directions.
assert((!ForceTopDown || !ForceBottomUp) &&
"-misched-topdown incompatible with -misched-bottomup");
if (ForceBottomUp.getNumOccurrences() > 0) {
RegionPolicy.OnlyBottomUp = ForceBottomUp;
if (RegionPolicy.OnlyBottomUp)
RegionPolicy.OnlyTopDown = false;
}
if (ForceTopDown.getNumOccurrences() > 0) {
RegionPolicy.OnlyTopDown = ForceTopDown;
if (RegionPolicy.OnlyTopDown)
RegionPolicy.OnlyBottomUp = false;
if (PreRADirection == MISched::TopDown) {
RegionPolicy.OnlyTopDown = true;
RegionPolicy.OnlyBottomUp = false;
} else if (PreRADirection == MISched::BottomUp) {
RegionPolicy.OnlyTopDown = false;
RegionPolicy.OnlyBottomUp = true;
} else if (PreRADirection == MISched::Bidirectional) {
RegionPolicy.OnlyBottomUp = false;
RegionPolicy.OnlyTopDown = false;
}
}

Expand Down Expand Up @@ -3911,17 +3907,15 @@ void PostGenericScheduler::initPolicy(MachineBasicBlock::iterator Begin,
MF.getSubtarget().overridePostRASchedPolicy(RegionPolicy, NumRegionInstrs);

// After subtarget overrides, apply command line options.
if (PostRADirection.getNumOccurrences() > 0) {
if (PostRADirection == MISchedPostRASched::TopDown) {
RegionPolicy.OnlyTopDown = true;
RegionPolicy.OnlyBottomUp = false;
} else if (PostRADirection == MISchedPostRASched::BottomUp) {
RegionPolicy.OnlyTopDown = false;
RegionPolicy.OnlyBottomUp = true;
} else if (PostRADirection == MISchedPostRASched::Bidirectional) {
RegionPolicy.OnlyBottomUp = false;
RegionPolicy.OnlyTopDown = false;
}
if (PostRADirection == MISched::TopDown) {
RegionPolicy.OnlyTopDown = true;
RegionPolicy.OnlyBottomUp = false;
} else if (PostRADirection == MISched::BottomUp) {
RegionPolicy.OnlyTopDown = false;
RegionPolicy.OnlyBottomUp = true;
} else if (PostRADirection == MISched::Bidirectional) {
RegionPolicy.OnlyBottomUp = false;
RegionPolicy.OnlyTopDown = false;
}
}

Expand Down Expand Up @@ -4368,10 +4362,9 @@ class InstructionShuffler : public MachineSchedStrategy {
} // end anonymous namespace

static ScheduleDAGInstrs *createInstructionShuffler(MachineSchedContext *C) {
bool Alternate = !ForceTopDown && !ForceBottomUp;
bool TopDown = !ForceBottomUp;
assert((TopDown || !ForceTopDown) &&
"-misched-topdown incompatible with -misched-bottomup");
bool Alternate =
PreRADirection != MISched::TopDown && PreRADirection != MISched::BottomUp;
bool TopDown = PreRADirection != MISched::BottomUp;
return new ScheduleDAGMILive(
C, std::make_unique<InstructionShuffler>(Alternate, TopDown));
}
Expand Down
7 changes: 2 additions & 5 deletions llvm/lib/CodeGen/VLIWMachineScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,6 @@ void ConvergingVLIWScheduler::initialize(ScheduleDAGMI *dag) {
HighPressureSets[i] =
((float)MaxPressure[i] > ((float)Limit * RPThreshold));
}

assert((!ForceTopDown || !ForceBottomUp) &&
"-misched-topdown incompatible with -misched-bottomup");
}

VLIWResourceModel *ConvergingVLIWScheduler::createVLIWResourceModel(
Expand Down Expand Up @@ -954,7 +951,7 @@ SUnit *ConvergingVLIWScheduler::pickNode(bool &IsTopNode) {
return nullptr;
}
SUnit *SU;
if (ForceTopDown) {
if (PreRADirection == MISched::TopDown) {
SU = Top.pickOnlyChoice();
if (!SU) {
SchedCandidate TopCand;
Expand All @@ -965,7 +962,7 @@ SUnit *ConvergingVLIWScheduler::pickNode(bool &IsTopNode) {
SU = TopCand.SU;
}
IsTopNode = true;
} else if (ForceBottomUp) {
} else if (PreRADirection == MISched::BottomUp) {
SU = Bot.pickOnlyChoice();
if (!SU) {
SchedCandidate BotCand;
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/dump-schedule-trace.mir
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=cortex-a55 \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler -o - %s \
# RUN: -misched-topdown=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=topdown -sched-print-cycles=true \
# RUN: -misched-dump-schedule-trace=true -misched-dump-schedule-trace-col-header-width=21 \
# RUN: 2>&1 | FileCheck %s --check-prefix=TOP --strict-whitespace

# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=cortex-a55 \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler -o - %s \
# RUN: -misched-bottomup=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=bottomup -sched-print-cycles=true \
# RUN: -misched-dump-schedule-trace=true -misched-dump-schedule-trace-col-width=4 \
# RUN: 2>&1 | FileCheck %s --check-prefix=BOTTOM --strict-whitespace

Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/force-enable-intervals.mir
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=cortex-a55 \
# RUN: -misched-dump-reserved-cycles=true \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler \
# RUN: -o - %s 2>&1 -misched-topdown| FileCheck %s
# RUN: -o - %s 2>&1 -misched-prera-direction=topdown | FileCheck %s

# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=cortex-a55 \
# RUN: -misched-dump-reserved-cycles=true -sched-model-force-enable-intervals=true \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler \
# RUN: -o - %s 2>&1 -misched-topdown| FileCheck %s --check-prefix=FORCE
# RUN: -o - %s 2>&1 -misched-prera-direction=topdown | FileCheck %s --check-prefix=FORCE

# REQUIRES: asserts, aarch64-registered-target
---
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -mattr=+neon -mcpu=cortex-a55 %s -o - 2>&1 \
# RUN: -misched-dump-reserved-cycles=true \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler \
# RUN: -misched-bottomup=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=bottomup -sched-print-cycles=true \
# RUN: -misched-detail-resource-booking=true \
# RUN: -misched-dump-schedule-trace=true -misched-dump-schedule-trace-col-header-width=21 \
# RUN: | FileCheck %s
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=cortex-a55 \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler -o - %s \
# RUN: -misched-bottomup=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=bottomup -sched-print-cycles=true \
# RUN: -misched-dump-reserved-cycles=true -misched-detail-resource-booking=true\
# RUN: -misched-dump-schedule-trace=true -misched-dump-schedule-trace-col-width=4 \
# RUN: 2>&1 | FileCheck %s
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/misched-sort-resource-in-trace.mir
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=exynos-m3 -verify-machineinstrs \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler -o - %s \
# RUN: -misched-topdown=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=topdown -sched-print-cycles=true \
# RUN: -misched-dump-schedule-trace=true --misched-sort-resources-in-trace=true 2>&1 | FileCheck --check-prefix=SORTED %s

# RUN: llc -mtriple=aarch64-none-linux-gnu -mcpu=exynos-m3 -verify-machineinstrs \
# RUN: -run-pass=machine-scheduler -debug-only=machine-scheduler -o - %s \
# RUN: -misched-topdown=true -sched-print-cycles=true \
# RUN: -misched-prera-direction=topdown -sched-print-cycles=true \
# RUN: -misched-dump-schedule-trace=true --misched-sort-resources-in-trace=false 2>&1 | FileCheck --check-prefix=UNSORTED %s

# REQUIRES: asserts, aarch64-registered-target
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/CodeGen/ARM/single-issue-r52.mir
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52 -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-topdown 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=TOPDOWN
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52 -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-bottomup 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOTTOMUP
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52plus -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-topdown 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=TOPDOWN
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52plus -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-bottomup 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOTTOMUP
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52 -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-prera-direction=topdown 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=TOPDOWN
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52 -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-prera-direction=bottomup 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOTTOMUP
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52plus -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-prera-direction=topdown 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=TOPDOWN
# RUN: llc -o /dev/null %s -mtriple=arm-eabi -mcpu=cortex-r52plus -run-pass machine-scheduler -enable-misched -debug-only=machine-scheduler -misched-prera-direction=bottomup 2>&1 | FileCheck %s --check-prefix=CHECK --check-prefix=BOTTOMUP
# REQUIRES: asserts
--- |
; ModuleID = 'foo.ll'
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/RISCV/sifive7-enable-intervals.mir
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# RUN: llc -mtriple=riscv64 -mcpu=sifive-x280 -run-pass=machine-scheduler \
# RUN: -debug-only=machine-scheduler -misched-dump-schedule-trace \
# RUN: -misched-topdown -o - %s 2>&1 | FileCheck %s
# RUN: -misched-prera-direction=topdown -o - %s 2>&1 | FileCheck %s
# REQUIRES: asserts

# The purpose of this test is to show that the VADD instructions are issued so
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/X86/handle-move.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: llc -mtriple=x86_64-- -mcpu=core2 -fast-isel -enable-misched -misched=shuffle -misched-bottomup -verify-machineinstrs < %s
; RUN: llc -mtriple=x86_64-- -mcpu=core2 -fast-isel -enable-misched -misched=shuffle -misched-topdown -verify-machineinstrs < %s
; RUN: llc -mtriple=x86_64-- -mcpu=core2 -fast-isel -enable-misched -misched=shuffle -misched-prera-direction=bottomup -verify-machineinstrs < %s
; RUN: llc -mtriple=x86_64-- -mcpu=core2 -fast-isel -enable-misched -misched=shuffle -misched-prera-direction=topdown -verify-machineinstrs < %s
; REQUIRES: asserts
;
; Test the LiveIntervals::handleMove() function.
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/X86/misched-aa-colored.ll
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
; RUN: llc < %s -mcpu=x86-64 -enable-misched -misched-bottomup=0 -misched-topdown=0 -misched=shuffle -enable-aa-sched-mi | FileCheck %s
; RUN: llc < %s -mcpu=x86-64 -enable-misched -misched-prera-direction=bidirectional -misched=shuffle -enable-aa-sched-mi | FileCheck %s
; REQUIRES: asserts
; -misched=shuffle is NDEBUG only!

Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/X86/misched-matrix.ll
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; RUN: llc < %s -mtriple=x86_64-- -mcpu=generic -pre-RA-sched=source -enable-misched \
; RUN: -misched-topdown -verify-machineinstrs \
; RUN: -misched-prera-direction=topdown -verify-machineinstrs \
; RUN: | FileCheck %s -check-prefix=TOPDOWN
; RUN: llc < %s -mtriple=x86_64-- -mcpu=generic -pre-RA-sched=source -enable-misched \
; RUN: -misched=ilpmin -verify-machineinstrs \
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/X86/misched-new.ll
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
; RUN: llc < %s -mtriple=x86_64-- -mcpu=core2 -x86-early-ifcvt -enable-misched \
; RUN: -misched=shuffle -misched-bottomup -verify-machineinstrs \
; RUN: -misched=shuffle -misched-prera-direction=bottomup -verify-machineinstrs \
; RUN: | FileCheck %s
; RUN: llc < %s -mtriple=x86_64-- -mcpu=core2 -x86-early-ifcvt -enable-misched \
; RUN: -misched=shuffle -misched-topdown -verify-machineinstrs \
; RUN: -misched=shuffle -misched-prera-direction=topdown -verify-machineinstrs \
; RUN: | FileCheck %s --check-prefix TOPDOWN
; REQUIRES: asserts
;
Expand Down
Loading