-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[TVMScript] Reorganize the folder structure #12496
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
[TVMScript] Reorganize the folder structure #12496
Conversation
Hzfengsy
left a comment
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
Hzfengsy
left a comment
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 think a better approach is renaming the new parser rather than the old one. i.e. we upstream at python/tvm/script/new_parser for now. Once the new parser is fully upstreamed, we rename it back
|
Okay either way works |
b2eceac to
2a199e4
Compare
52afa15 to
c26ced4
Compare
0c71ac1 to
f13c52b
Compare
75f1bcc to
53ead89
Compare
|
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 |
71df867 to
61ba89b
Compare
tests/python/unittest/test_tir_transform_lower_cross_thread_reduction.py
Show resolved
Hide resolved
This PR introduces some minor restructuring of the `python/tvm/script` folder structure to make it more convenient for future upstreaming. Co-authored-by: Yaxing Cai <caiyaxing666@gmail.com>
61ba89b to
23b8b0b
Compare
MasterJH5574
left a comment
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.
Thanks for resolving my comments! LGTM.
For the previous version of the parser, this was special-cased for some intrinsic operators. After the new TVMScript was enabled in apache#12496, any `PrimExpr` that appears in the body of a statement is silently ignored. This commit updates the parser to instead wrap the bare `PrimExpr` in a `tir::Evaluate` node. This change effectively allows [expression statements](https://docs.python.org/3/reference/simple_stmts.html#expression-statements) in TVMScript, which are converted to `tir::Evaluate` nodes during parsing.
|
|
||
| from .parser import ir_module, from_source | ||
| """TVM Script APIs of TVM Python Package""" | ||
| from .parser import ir, ir_module, parse as from_source, tir |
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.
@junrushao it appears this line, combined with the movement of the old parser from parser/ to /parser_v1 means that this PR implicitly switches things over to using the new parser. Am I interpreting this correctly, and was that intentional?
…13396) * [TVMScript] Use tir::Evaluate if expression is in statement context For the previous version of the parser, this was special-cased for some intrinsic operators. After the new TVMScript was enabled in #12496, any `PrimExpr` that appears in the body of a statement is silently ignored. This commit updates the parser to instead wrap the bare `PrimExpr` in a `tir::Evaluate` node. This change effectively allows [expression statements](https://docs.python.org/3/reference/simple_stmts.html#expression-statements) in TVMScript, which are converted to `tir::Evaluate` nodes during parsing. * Update to print T.evaluate() for readability, except for CallNode
This PR introduces some minor restructuring of the `python/tvm/script` folder structure to make it more convenient for future upstreaming. Co-authored-by: Yaxing Cai <caiyaxing666@gmail.com>
…pache#13396) * [TVMScript] Use tir::Evaluate if expression is in statement context For the previous version of the parser, this was special-cased for some intrinsic operators. After the new TVMScript was enabled in apache#12496, any `PrimExpr` that appears in the body of a statement is silently ignored. This commit updates the parser to instead wrap the bare `PrimExpr` in a `tir::Evaluate` node. This change effectively allows [expression statements](https://docs.python.org/3/reference/simple_stmts.html#expression-statements) in TVMScript, which are converted to `tir::Evaluate` nodes during parsing. * Update to print T.evaluate() for readability, except for CallNode
Usually, when using TVMScript to represent a `PrimFunc` variable definition `var_name = expr` defines `LetStmt` with a variable named `var_name` bound to the expression `expr`. However, prior to this commit, if `expr` is a `tir::Var`, the TVMScript parser would instead silently omit the `LetStmt`, and rename all instances of that variable to `var_name`. The root cause was in the `VarTable.exist` check, which erroneously returned False in all cases. This was due to a `value is v` check, which checked if the value was the same as the stack of maybe-shadowing values that share the same name. Replacing the 'value is v` check with a `value in v` check resolves this issue. This bug dates to the initial implementation of the new TVMScript parser in apache#12496.
) * [Bugfix][TVMScript] Handle LetStmt for `var1 = var2` expressions Usually, when using TVMScript to represent a `PrimFunc` variable definition `var_name = expr` defines `LetStmt` with a variable named `var_name` bound to the expression `expr`. However, prior to this commit, if `expr` is a `tir::Var`, the TVMScript parser would instead silently omit the `LetStmt`, and rename all instances of that variable to `var_name`. The root cause was in the `VarTable.exist` check, which erroneously returned False in all cases. This was due to a `value is v` check, which checked if the value was the same as the stack of maybe-shadowing values that share the same name. Replacing the 'value is v` check with a `value in v` check resolves this issue. This bug dates to the initial implementation of the new TVMScript parser in #12496. * Avoid implicit `PrimExpr.__bool__` from `if value in value_stack` * Use T.meta_var where variable renaming is required.
…che#14320) * [Bugfix][TVMScript] Handle LetStmt for `var1 = var2` expressions Usually, when using TVMScript to represent a `PrimFunc` variable definition `var_name = expr` defines `LetStmt` with a variable named `var_name` bound to the expression `expr`. However, prior to this commit, if `expr` is a `tir::Var`, the TVMScript parser would instead silently omit the `LetStmt`, and rename all instances of that variable to `var_name`. The root cause was in the `VarTable.exist` check, which erroneously returned False in all cases. This was due to a `value is v` check, which checked if the value was the same as the stack of maybe-shadowing values that share the same name. Replacing the 'value is v` check with a `value in v` check resolves this issue. This bug dates to the initial implementation of the new TVMScript parser in apache#12496. * Avoid implicit `PrimExpr.__bool__` from `if value in value_stack` * Use T.meta_var where variable renaming is required.
…che#14320) * [Bugfix][TVMScript] Handle LetStmt for `var1 = var2` expressions Usually, when using TVMScript to represent a `PrimFunc` variable definition `var_name = expr` defines `LetStmt` with a variable named `var_name` bound to the expression `expr`. However, prior to this commit, if `expr` is a `tir::Var`, the TVMScript parser would instead silently omit the `LetStmt`, and rename all instances of that variable to `var_name`. The root cause was in the `VarTable.exist` check, which erroneously returned False in all cases. This was due to a `value is v` check, which checked if the value was the same as the stack of maybe-shadowing values that share the same name. Replacing the 'value is v` check with a `value in v` check resolves this issue. This bug dates to the initial implementation of the new TVMScript parser in apache#12496. * Avoid implicit `PrimExpr.__bool__` from `if value in value_stack` * Use T.meta_var where variable renaming is required.
This PR introduces some minor restructuring of the
python/tvm/scriptfolder structure to make it more convenient for future upstreaming.
CC: @Hzfengsy @cyx-6