Skip to content
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

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

Merged
merged 1 commit into from
Dec 11, 2023

Conversation

shining1984
Copy link
Contributor

The member functions of ScheduleDAGMI are called back from PostMachineScheduler::runOnMachineFunction, instead of MachineScheduler::runOnMachineFunction.

The member functions of ScheduleDAGMI are called back from
PostMachineScheduler::runOnMachineFunction, instead of
MachineScheduler::runOnMachineFunction.
/// 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

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.

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the detailed explanation!

@ChunyuLiao ChunyuLiao merged commit fc715e4 into llvm:main Dec 11, 2023
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants