-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[bug] [ir] Fix Block::replace_with segment faults when stmts.size() == 0 #1072
Conversation
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.
Looks good, but I think we'd better modify replace_with
rather than void visit(FrontendEvalStmt *stmt)
to prevent other potential bugs. I suggest changing
if (replace_usages)
old_statement->replace_with(new_statements.back().get());
to
if (replace_usages && !new_statements.empty())
old_statement->replace_with(new_statements.back().get());
BTW I think it weird when new_statements.size() != 1 && replace_usages == true
. Do we need this design in any IR passes?
Hello? @yuanming-hu I know you're busy preparing GAMES201, so can I merge this omo once @xumingkuan approved? There are now 14 PRs waiting for merge, I'll merge some with trival changeset omo with 1 approve from taichi-dev write-members. |
Sure, having Mingkuan's approval is good enough. Btw, please capitalize the first letter of your PR letter and avoid confusing abbreviations. Thank you! |
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.
Cool!
(Actually we can add |
Related issue = close #1071
[Click here for the format server]
Seems
replace_usage = true
not working whennew_statements.size() == 0
:taichi/taichi/ir/ir.cpp
Lines 760 to 761 in c193e4b
... where
new_statements.back() == nullptr
.