Skip to content

Conversation

@WuSiYu
Copy link
Contributor

@WuSiYu WuSiYu commented Oct 12, 2022

This commit fix the tvm's crash during auto-tune when having a 'if' statement which doesn't have a 'else' part in TVMScript.
It's cause in tvm's tir processing, in VisitStmt_() for IfThenElseNode its trying to visit the 'else' part (in TVMScript) whether it is exist.
So in this commit, an if logic is added in VisitStmt_() for IfThenElseNode, it will not visit the 'else' part if it is not exist.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Oct 12, 2022

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

Copy link

@shingjan shingjan left a comment

Choose a reason for hiding this comment

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

Thanks for sending this! Looks fine to me. Would that be possible that some tests can be added to this PR so we will know if future changes will break this?

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

LGTM

@WuSiYu WuSiYu force-pushed the main-fix-tvmscript-if-without-else branch from 13ff8eb to 0127232 Compare October 13, 2022 08:00
@WuSiYu
Copy link
Contributor Author

WuSiYu commented Oct 13, 2022

Would that be possible that some tests can be added to this PR so we will know if future changes will break this?

Yes, a test has been added in test_tir_analysis_estimate_tir_flops.py

Copy link

@shingjan shingjan left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test!

@WuSiYu WuSiYu force-pushed the main-fix-tvmscript-if-without-else branch from 0127232 to 5370454 Compare October 13, 2022 08:18
@WuSiYu
Copy link
Contributor Author

WuSiYu commented Oct 13, 2022

@tvm-bot rerun

@WuSiYu WuSiYu force-pushed the main-fix-tvmscript-if-without-else branch from 5370454 to b98a87c Compare October 13, 2022 14:03
@WuSiYu
Copy link
Contributor Author

WuSiYu commented Oct 15, 2022

I am not sure but it seem's like the CI is failed by the other ramdom reason, should I continue to try to rerun it?

@shingjan
Copy link

shingjan commented Oct 17, 2022

@WuSiYu It seems that the @tvm-bot rerun doesn't go through for some reason. The last time CI failed for this PR is for this test, which has been reported and fixed by #13067. Feel free to rebase and retrigger CI.

This commit fix the tvm's crash during TVMScript's auto-tune when having a
'if' statement which doesn't have a 'else' part.
It's cause in tvm's tir processing (tvm.tir.analysis.estimate_flops), in
VisitStmt_() for IfThenElseNode it trying to visit the 'else' part whether
it is exist.
So in this commit, an if logic is added in VisitStmt_() for
IfThenElseNode, it will not visit the 'else' part if it is not exist.
@WuSiYu WuSiYu force-pushed the main-fix-tvmscript-if-without-else branch from b98a87c to 5f95c0f Compare October 17, 2022 03:41
@junrushao junrushao merged commit 69ba30e into apache:main Oct 17, 2022
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 10, 2022
…che#13054)

This commit fix the tvm's crash during TVMScript's auto-tune when having a
'if' statement which doesn't have a 'else' part.
It's cause in tvm's tir processing (tvm.tir.analysis.estimate_flops), in
VisitStmt_() for IfThenElseNode it trying to visit the 'else' part whether
it is exist.
So in this commit, an if logic is added in VisitStmt_() for
IfThenElseNode, it will not visit the 'else' part if it is not exist.

Co-authored-by: SiYu Wu <wusiyu@buaa.edu.cn>
xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022
…che#13054)

This commit fix the tvm's crash during TVMScript's auto-tune when having a
'if' statement which doesn't have a 'else' part.
It's cause in tvm's tir processing (tvm.tir.analysis.estimate_flops), in
VisitStmt_() for IfThenElseNode it trying to visit the 'else' part whether
it is exist.
So in this commit, an if logic is added in VisitStmt_() for
IfThenElseNode, it will not visit the 'else' part if it is not exist.

Co-authored-by: SiYu Wu <wusiyu@buaa.edu.cn>
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.

5 participants