-
Notifications
You must be signed in to change notification settings - Fork 4k
[TIR] Change the behavior of read/write region analysis for reduction blocks. #10638
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
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
d5ef430
upd
yzh119 7fe6228
fix
yzh119 d826300
fix rfactor
yzh119 2684364
fix blockize
yzh119 8805c1e
tensorize
yzh119 e1a7c50
upd
yzh119 9dc8273
fix meta-schedules
yzh119 c641c30
upd
yzh119 d268ba0
fix argmax
yzh119 9a59bdc
manally call region analysis
yzh119 cd42ab1
Merge branch 'main' into reduction-block-reads
yzh119 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After this PR, I'm getting a structural hash mismatch in meta schedule
ApplyHistoryBesttvm/src/meta_schedule/integration.cc
Line 134 in ce335c3
I found that if I do
decompose_reduction, IRModules that have been committed to the database are modified. Specifically, the query module hasbut the corresponding mod in the database has different
reads:I haven't looked at what this PR does, but I'm pretty sure this line is the offending bug...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@masahi thanks for reporting this! I'm not sure why the IRModule is mutated in place after decompose-reduction (not sure if i understand correctly), but would be great to have a minimal reproducible example after you returning from vacation :-)
@yzh119 would you mind taking a look at the particular case Masa mentions and perhaps add a regression test?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I can prepare a repro. It's a bit complicated since it involves scheduling, database stuff, and
ApplyHistoryBest. I know the issue comes from this PR since I bisected it.So when
decompose_reductionis called, we are now executinghttps://github.com/yzh119/tvm/blob/cd42ab101ab993edae2956fdc0415561be5f9718/src/tir/schedule/primitive/reduction.cc#L301
and I'm guessing that this in-place update propagates all the way up to the committed
prim_modin the tuning database.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok the repro is ready: https://gist.github.com/masahi/591078723d26f09ece3430af95835c99
The test works with the current
main. When I run it, I get this output:This means, if
decompose_reductionis applied during scheduling,ApplyHistoryBestfails to match the structural hash of the query mod, corresponding tonn_contrib_denseandnn_batch_matmul, against the ones in database.If I revert this commit, I get the following expected output
You can dump the contents of the database at
tvm/src/meta_schedule/database/json_database.cc
Lines 72 to 74 in b08e8e4
readsregion for dense and bmm is modified ifdecompose_reductionis enabled. This causes the structual hash mismatch between the modules in the database and the query mod.