-
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
[opt] Simplify replace_statements and improve demote_dense_struct_fors #2335
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.
LGTM!
taichi/ir/transforms.h
Outdated
bool transform_statements( | ||
IRNode *root, | ||
std::function<bool(Stmt *)> filter, | ||
std::function<void(Stmt *, DelayedIRModifier *)> transformer); | ||
/** | ||
* @param root The IR root to be traversed. | ||
* @param filter A function which tells if a statement need to be replaced. | ||
* @param generator If a statement s need to be replaced, generate a new | ||
* statement s1 with the argument s, insert s1 to s's place, and replace all |
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.
nit: by saying "insert s1 to s's place", do we mean "insert |s1| to where |s| is defined, removes |s|'s definition and replaces all usages of ..."?
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.
Right, I'll rephrase this.
bool replace_statements_with(IRNode *root, | ||
std::function<bool(Stmt *)> filter, | ||
std::function<Stmt *(Stmt *)> generator); | ||
bool replace_statements(IRNode *root, |
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.
Following the above comment, how is |s|'s defining stmt handled in this case?
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.
|s|'s defining stmt is erased here.
Related issue = #1989 -- This PR provides a filtered map operation on the IR. We can replace some
gather_statements
usages withtransform_statements
or something similar.A continuation of #2306.
Also reduces the time complexity of demote_dense_struct_fors from O(snodes.size() * (physical_indices.size() + N)) to O(snodes.size() * physical_indices.size() + N) where N is the total number of statements in the IR.
[Click here for the format server]