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

[Relax] Apply DefaultGPUSchedule() in default build pipeline #17108

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Lunderberg
Copy link
Contributor

This is a follow-up to #15864, which added LegalizeOps to the default Relax build pipeline. Since legalization may produce additional TIR PrimFuncs that require scheduling, the output of LegalizeOps typically must also be passed through tir.transform.DefaultGPUSchedule(). This PR adds DefaultGPUSchedule() to the relax build pipeline to handle these cases.

Scheduled PrimFunc have the "tir.is_scheduled" attribute set to true, and are ignored by DefaultGPUSchedule(). In addition, the DefaultGPUSchedule transform has no effect on non-GPU targets. Therefore, this change should only impact tvm.relax.build calls that previously resulted in an error due to unscheduled GPU functions, and should not have any impact on existing calls.

This is a follow-up to apache#15864, which
added `LegalizeOps` to the default Relax build pipeline.  Since
legalization may produce additional TIR PrimFuncs that require
scheduling, the output of `LegalizeOps` typically must also be passed
through `tir.transform.DefaultGPUSchedule()`.  This PR adds
`DefaultGPUSchedule()` to the relax build pipeline to handle these
cases.

Scheduled PrimFunc have the `"tir.is_scheduled"` attribute set to
true, and are ignored by `DefaultGPUSchedule()`.  In addition, the
`DefaultGPUSchedule` transform has no effect on non-GPU targets.
Therefore, this change should only impact `tvm.relax.build` calls that
previously resulted in an error due to unscheduled GPU functions, and
should not have any impact on existing calls.
@yongwww
Copy link
Member

yongwww commented Jun 18, 2024

Seems we discussed this before in the phase ordering discussion in open dev meeting, can't remember the reason why we didn't add this pass in default pipeline, need to take a look at the meeting note

@Lunderberg
Copy link
Contributor Author

I don't remember that discussion, but I'd be interested in seeing the minutes from it. The main roadblocks to it are ones that I've resolved in previous PRs:

With those fixes applied, I think it's now a pretty big usability benefit to include it in the lowering pipeline.

@MadFunMaker
Copy link

MadFunMaker commented Aug 1, 2024

LGTM - I think it's clear usability gain without significant extra burden on build pipeline.

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

3 participants