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

[TE] Record primitives of Schedule for visualization #14168

Merged
merged 3 commits into from
Mar 14, 2023

Conversation

chunit-quic
Copy link
Contributor

  • Make Stage link to its Schedule via attach_sch.
  • Add two array attributes, primitive_record and schedule_record to Schedule.
  • Create a new class, ScheduleContext, to record primitives.
  • Register a pass config variable, keep_schedule_record to enable/disable the recording.
  • Add test cases for TEDD, build_module and schedule ops.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Mar 2, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

Generated by tvm-bot

@chunit-quic
Copy link
Contributor Author

Hi community,

This is second part of TVM Explorer infrastructures. After this patch, user can backtrack what
(schedule) primitives have been applied to a schedule. Pair with the tool, TEDD, these changes can
be visualized beautifully.

To give you more context, we briefly describe what have been done below in this PR. Here is the
list of main modifications:

  1. Make a Stage know which Schedule it belongs to.
  2. Add two attributes, primitive_record and schedule_record to Schedule.
  3. Create a new class, ScheduleContext, to record primitive in the RAII way.
  4. Register a pass config variable, keep_schedule_record to enable/disable the recording.

Everytime a Stage being modified by a primitive, we will use ScheduleContext to record the optimized schedule and corresponding primitive. So that we can obtain the modification sequence later. You can reference to preRFC for more details. Thanks!

For your reference, here is the related links.
PreRFC in forum
apache/tvm-rfcs#92
#13116
@haowhsu-quic, @zack-ch

@chunit-quic chunit-quic force-pushed the ScheduleVisualization branch from fd4b1da to 3547d63 Compare March 2, 2023 08:28
Copy link
Member

@Hzfengsy Hzfengsy left a comment

Choose a reason for hiding this comment

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

LGTM. But I'm not confident to explicitly approve it since I'm not an expert on TE

@chunit-quic chunit-quic force-pushed the ScheduleVisualization branch 3 times, most recently from 78c3d62 to 8b14890 Compare March 9, 2023 01:49
- Make Stage link to its Schedule via attach_sch.
- Add two array attributes, primitive_record and schedule_record to Schedule.
- Create a new class, ScheduleContext, to record primitives.
- Register a pass config variable, keep_schedule_record to enable/disable the recording.
- Add test cases for TEDD, build_module and schedule ops.
@chunit-quic chunit-quic force-pushed the ScheduleVisualization branch from 8b14890 to 1c5ce66 Compare March 9, 2023 07:26
@chunit-quic
Copy link
Contributor Author

Hi @Hzfengsy,

LGTM. But I'm not confident to explicitly approve it since I'm not an expert on TE

Thank you for reviewing! May I know is there any recommended person who could help to review this topic? :)

Hi @masahi,
Sorry to bother you, because I saw you dealt with some TE related PRs, and had checked the RFC of Explorer before. Would you mind to take a look at this PR and give us some feedbacks? Thank you. :D

Copy link
Member

@Hzfengsy Hzfengsy left a comment

Choose a reason for hiding this comment

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

Thanks, @chunit-quic for the PR. I explicitly approve this PR after another look. Let's merge it in 24h if there are no other objectives.

@chunit-quic
Copy link
Contributor Author

Thanks, @chunit-quic for the PR. I explicitly approve this PR after another look. Let's merge it in 24h if there are no other objectives.

No problem. Thanks a lot for helping us! :D

Copy link
Member

@masahi masahi left a comment

Choose a reason for hiding this comment

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

Please fix basic grammar issues.

include/tvm/te/schedule.h Outdated Show resolved Hide resolved
include/tvm/te/schedule.h Outdated Show resolved Hide resolved
include/tvm/te/schedule.h Show resolved Hide resolved
python/tvm/contrib/tedd.py Outdated Show resolved Hide resolved
include/tvm/te/schedule.h Outdated Show resolved Hide resolved
* Fix grammar issues
* Rewrite unclear comments
@chunit-quic
Copy link
Contributor Author

Please fix basic grammar issues.

Hi @masahi,

Thank you very much for reviewing!
We're pretty sorry for basic issues of documentation.
We have rewritten those parts in comments. If there is still anything unclear we would try to reword them at once.

Thank you!

* Remove the wrong term, rebased in comments and variables.
@masahi masahi merged commit ce1fa89 into apache:main Mar 14, 2023
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.

4 participants