-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[TIR] [Bugfix] Pass the correct block_sref_reuse to Replace #14023
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
Conversation
|
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 |
A mismatch between the blocks present in the `result` vs the blocks passed in `block_sref_to_reuse` caused the bug mentioned in apache#13974. This patch tries to fix that bug by collecting only the blocks that are part of result and also present in the block replacement map `new_block_to_old_`. Since the scope block is `result`, only that block and its child blocks would be replaced, and any replaced block would be present in `rewriter.new_block_to_old_`. Thus, collecting the replaced blocks from among child blocks of `result` guarantees that the `block_sref_reuse` would contain all the replaced blocks and that they'll point to the correct block in `result` thus avoiding the missing SRef error.
fbb30f0 to
832ffdc
Compare
Hzfengsy
left a comment
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.
Please add a regression test for it
Yes, I realized that I did not add the test after creating the PR. I'll update with the test, thanks. |
Hzfengsy
left a comment
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.
LGTM. Thanks @quic-sanirudh
…4023) * [TIR] [Bugfix] Pass the correct block_sref_reuse to Replace A mismatch between the blocks present in the `result` vs the blocks passed in `block_sref_to_reuse` caused the bug mentioned in apache#13974. This patch tries to fix that bug by collecting only the blocks that are part of result and also present in the block replacement map `new_block_to_old_`. Since the scope block is `result`, only that block and its child blocks would be replaced, and any replaced block would be present in `rewriter.new_block_to_old_`. Thus, collecting the replaced blocks from among child blocks of `result` guarantees that the `block_sref_reuse` would contain all the replaced blocks and that they'll point to the correct block in `result` thus avoiding the missing SRef error.
A mismatch between the blocks present in the
resultvs the blocks passed inblock_sref_to_reusecaused the bug mentioned in #13974.This patch tries to fix that bug by collecting only the blocks that are part of result and also present in the block replacement map
new_block_to_old_. Since the scope block isresult, only that block and its child blocks would be replaced, and any replaced block would be present inrewriter.new_block_to_old_. Thus, collecting the replaced blocks from among child blocks ofresultguarantees that theblock_sref_reusewould contain all the replaced blocks and that they'll point to the correct block inresultthus avoiding the missing SRef error.