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
[IR] Add inline docs to all statements #2276
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!
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.
I took another look. Please help resolve the variable quoting style, thanks!
taichi/ir/statements.h
Outdated
@@ -147,6 +165,9 @@ class RandStmt : public Stmt { | |||
TI_DEFINE_ACCEPT_AND_CLONE | |||
}; | |||
|
|||
/** | |||
* A binary operation. TODO: remove the field is_bit_vectorized. |
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.
- Move TODO to its own line, 2) properly quote the variable
@@ -174,6 +195,10 @@ class BinaryOpStmt : public Stmt { | |||
TI_DEFINE_ACCEPT_AND_CLONE | |||
}; | |||
|
|||
/** | |||
* A ternary operation. Currently "select" (the ternary conditional operator, |
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.
Pls unify the variable quoting style
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.
Oh this is not a variable :-)
taichi/ir/statements.h
Outdated
@@ -225,6 +256,9 @@ class ExternalPtrStmt : public Stmt { | |||
TI_DEFINE_ACCEPT_AND_CLONE | |||
}; | |||
|
|||
/** | |||
* A global pointer. |
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.
A bit more explanation on this (on a new paragraph)? For example, this will not show up in the final lowered IR.
taichi/ir/statements.h
Outdated
@@ -1041,11 +1191,15 @@ class ClearListStmt : public Stmt { | |||
// Checks if the task represented by |stmt| contains a single ClearListStmt. | |||
bool is_clear_list_task(const OffloadedStmt *stmt); | |||
|
|||
/** | |||
* TODO: document for InternalFuncStmt |
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.
So... what's an InternalFuncStmt
?
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.
It seems to be used by ti.call_internal
, which is to run some C++ tests in taichi/runtime/llvm/internal_functions.h
.
Let's put a TODO saying that we'd like to remove this :)
Co-authored-by: Ye Kuang <k-ye@users.noreply.github.com>
@k-ye Thanks for the review! |
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.
This is great, thank you so much!
Related issue = #2193 #2192
This PR adds inline docs to all statements in statements.h.
[Click here for the format server]