Skip to content

[CodeGen][MachineScheduler][NFC]Update some comments of scheduler #74705

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.

Sign up for GitHub

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 11, 2023
Merged

[CodeGen][MachineScheduler][NFC]Update some comments of scheduler #74705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion llvm/include/llvm/CodeGen/SchedulerRegistry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class RegisterScheduler
ScheduleDAGSDNodes *createBURRListDAGScheduler(SelectionDAGISel *IS,
CodeGenOptLevel OptLevel);

/// createBURRListDAGScheduler - This creates a bottom up list scheduler that
/// createSourceListDAGScheduler - This creates a bottom up list scheduler that
/// schedules nodes in source code order when possible.
ScheduleDAGSDNodes *createSourceListDAGScheduler(SelectionDAGISel *IS,
CodeGenOptLevel OptLevel);
Expand Down
12 changes: 6 additions & 6 deletions llvm/lib/CodeGen/MachineScheduler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,9 @@ void ScheduleDAGMI::finishBlock() {
ScheduleDAGInstrs::finishBlock();
}

/// enterRegion - Called back from MachineScheduler::runOnMachineFunction after
/// crossing a scheduling boundary. [begin, end) includes all instructions in
/// the region, including the boundary itself and single-instruction regions
/// enterRegion - Called back from PostMachineScheduler::runOnMachineFunction
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this true? I think both PostMachineScheduler and MachineScheduler call this.
Should it be MachineSchedulerBase::scheduleRegions?

Copy link
Contributor Author

@shining1984 shining1984 Dec 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

  1. MachineScheduler::runOnMachineFunction and PostMachineScheduler::runOnMachineFunction all call the MachineSchedulerBase::scheduleRegions.
  2. Then MachineSchedulerBase::scheduleRegions call the Scheduler.enterRegion and Scheduler.schedule.
  3. The Scheduler in MachineSchedulerBase::scheduleRegions is from MachineScheduler::runOnMachineFunction or PostMachineScheduler::runOnMachineFunction.
  4. When Scheduler is from the MachineScheduler::runOnMachineFunction, it is return from the MachineScheduler::createMachineScheduler(), which get a new ScheduleDAGMILive pointer from the PassConfig->createMachineScheduler(this) or createGenericSchedLive. Then the Scheduler is ScheduleDAGMILive , the Scheduler.enterRegion is ScheduleDAGMILive.enterRegion. So, we can say the ScheduleDAGMILive::enterRegion is called back from MachineScheduler::runOnMachineFunction. The ScheduleDAGMILive::schedule is same as the ScheduleDAGMILive::enterRegion.
  5. When Scheduler is from the PostMachineScheduler::runOnMachineFunction, it is return from the PostMachineScheduler::createPostMachineScheduler(), which get a new ScheduleDAGMI pointer from the PassConfig->createPostMachineScheduler(this) or createGenericSchedPostRA. Then the Scheduler is ScheduleDAGMI , the Scheduler.enterRegion is ScheduleDAGMI.enterRegion. So, we can say the ScheduleDAGMI::enterRegion is called back from PostMachineScheduler::runOnMachineFunction.The ScheduleDAGMI::schedule is same as the ScheduleDAGMI::enterRegion.
  6. I changed the two comments from MachineScheduler::runOnMachineFunction to PostMachineScheduler::runOnMachineFunction is according the "5".

Copy link
Member

Choose a reason for hiding this comment

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

The convention for new code is "Don’t duplicate function or class name at the beginning of the comment." https://llvm.org/docs/CodingStandards.html#commenting

Copy link
Contributor

Choose a reason for hiding this comment

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

The convention for new code is "Don’t duplicate function or class name at the beginning of the comment." https://llvm.org/docs/CodingStandards.html#commenting

I think this is a fix for existed comments. :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The convention for new code is "Don’t duplicate function or class name at the beginning of the comment." https://llvm.org/docs/CodingStandards.html#commenting

This pr just fixs three existed comments.
There are many comments in the two files need to update, if follow the "Don’t duplicate function or class name at the beginning of the comment." Maybe need a new pr to update them.

/// after crossing a scheduling boundary. [begin, end) includes all instructions
/// in the region, including the boundary itself and single-instruction regions
/// that don't get scheduled.
void ScheduleDAGMI::enterRegion(MachineBasicBlock *bb,
MachineBasicBlock::iterator begin,
Expand Down Expand Up @@ -793,9 +793,9 @@ bool ScheduleDAGMI::checkSchedLimit() {
}

/// Per-region scheduling driver, called back from
/// MachineScheduler::runOnMachineFunction. This is a simplified driver that
/// does not consider liveness or register pressure. It is useful for PostRA
/// scheduling and potentially other custom schedulers.
/// PostMachineScheduler::runOnMachineFunction. This is a simplified driver
/// that does not consider liveness or register pressure. It is useful for
/// PostRA scheduling and potentially other custom schedulers.
void ScheduleDAGMI::schedule() {
LLVM_DEBUG(dbgs() << "ScheduleDAGMI::schedule starting\n");
LLVM_DEBUG(SchedImpl->dumpPolicy());
Expand Down