You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "Lunderberg (via GitHub)" <gi...@apache.org> on 2023/12/01 21:44:37 UTC
[PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Lunderberg opened a new pull request, #16195:
URL: https://github.com/apache/tvm/pull/16195
Prior to this commit, the normalizer printed a warning whenever a lowering pass produced a `SeqExpr` containing a non-dataflow `BindingBlock` within a `DataflowBlock`. This intermediate is only ill-formed if the non-dataflow `BindingBlock` makes use of any `DataflowVar` instances, as the `BindingBlock` can otherwise be hoisted out.
This commit explicitly checks for use of `DataflowVar` in these cases, as this is most likely due to erroneous use of `BindingBlock` where `DataflowBlock` should be used, and changes the severity from `WARNING` to `FATAL`. If there are no `DataflowVar` instances used in the `BindingBlock`, no warning is required.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Posted by "slyubomirsky (via GitHub)" <gi...@apache.org>.
slyubomirsky commented on PR #16195:
URL: https://github.com/apache/tvm/pull/16195#issuecomment-1850892168
I think this is a good improvement. Could you also please add a test case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg merged PR #16195:
URL: https://github.com/apache/tvm/pull/16195
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #16195:
URL: https://github.com/apache/tvm/pull/16195#issuecomment-1856041150
Can do, and I've added a few unit tests. The tests show which errors can be caught during normalization, which ones cannot, and the well-formed output that would erroneously be warned about if the error-checking were more stringent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #16195:
URL: https://github.com/apache/tvm/pull/16195#issuecomment-1856500519
Agreed on testing for error cases. I tend to add the tests when writing new checks, as I'm already in the process of writing the associated tests, and therefore have a way to drive the function. When updating/tweaking existing errors I frequently omit them, because it may not be easy/possible to write a test case using only the public APIs that exactly hits the specific error case.
(Granted, difficulty in writing specific tests is an issue in itself, and refactoring to allow better tests is a net positive. But I tend to go down those rabbit holes with disturbing regularity, and try to cut myself off where possible.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Unity] Improved error checking for DataflowBlock in nested SeqExpr [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #16195:
URL: https://github.com/apache/tvm/pull/16195#issuecomment-1836820582
This encountered while implementing https://github.com/apache/tvm/pull/16194. While not strictly required for that PR, the `LOG(WARNING)` on current unity head produces many spurious warnings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org