You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/12/02 08:06:12 UTC
[GitHub] [tvm] shingjan opened a new pull request #9634: [TVMScript] Syntax sugar for reads & writes
shingjan opened a new pull request #9634:
URL: https://github.com/apache/tvm/pull/9634
This PR intends to add syntax sugar for `T.reads` and `T.writes`
Before this PR:
```
T.reads([C[vi, vj], A_shared[vk, vi], B[vk, vj]])
T.writes([C[vi, vj]])
```
After this PR:
```
T.reads(C[vi, vj], A_shared[vk, vi], B[vk, vj])
T.writes(C[vi, vj])
```
cc: @vinx13 @junrushao1994 @Hzfengsy
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on a change in pull request #9634: [TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
vinx13 commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r763277900
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -210,9 +210,14 @@ def alloc_buffer(
"""
special_stmt - Reads/Writes
"""
-
+@overload
def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+@overload
+def reads(read_regions: BufferSlice, *other_regions: BufferSlice) -> None: ...
Review comment:
From the user-facing side, if we have two overloading versions I don't think `other_regions` is necessary. We can instead do something like
```
@overload
def reads(read_regions: List[BufferSlice]) -> None: ...
@overload
def reads(*read_regions: BufferSlice) -> None: ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-985032605
please also update type stubs (adding an overload like `reads(*read_regions: BufferSlice`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r762472910
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -211,8 +211,12 @@ def alloc_buffer(
special_stmt - Reads/Writes
"""
-def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
-def writes(write_region: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+def reads(
+ read_regions: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
+def writes(
+ write_region: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
Review comment:
Just curious, is it possible to use @overload to separate the logic more clearly?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-985929059
@vinx13 I fixed the CI error by removing the `test_mismatch_args` in `test_tvmscript_error_report` as the error condition of that test, `T.reads(A[...], A[...])` no longer holds after this PR. @Hzfengsy It would be great if you can take a look and confirm it. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 commented on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
vinx13 commented on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-985702420
please fix CI error
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-985270024
@vinx13 stub is updated
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #9634: [TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r763285293
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -210,9 +210,14 @@ def alloc_buffer(
"""
special_stmt - Reads/Writes
"""
-
+@overload
def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+@overload
+def reads(read_regions: BufferSlice, *other_regions: BufferSlice) -> None: ...
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan edited a comment on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan edited a comment on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-985929059
@vinx13 I fixed the CI error by removing the `test_mismatch_args` in `test_tvmscript_error_report` as the error condition of that test, `T.reads(A[...], A[...]) ----> error on mismatch args` no longer holds after this PR. @Hzfengsy It would be great if you can take a look and confirm it. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#issuecomment-986102486
Let's also improve the printer to print the sugared syntax by default :-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] vinx13 merged pull request #9634: [TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
vinx13 merged pull request #9634:
URL: https://github.com/apache/tvm/pull/9634
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #9634: [WIP][TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r763257688
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -211,8 +211,12 @@ def alloc_buffer(
special_stmt - Reads/Writes
"""
-def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
-def writes(write_region: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+def reads(
+ read_regions: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
+def writes(
+ write_region: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
Review comment:
using @overload in TIR script will cause an error as @overload will call an anonymous function which is not allowed currently [here](https://github.com/apache/tvm/blob/3f1c931aa1414b69103c8c3ca019a9f55b5cb7a7/python/tvm/script/utils.py#L47) in TVM.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #9634: [TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r763257688
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -211,8 +211,12 @@ def alloc_buffer(
special_stmt - Reads/Writes
"""
-def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
-def writes(write_region: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+def reads(
+ read_regions: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
+def writes(
+ write_region: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
Review comment:
Will update the pyi with @overload accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] shingjan commented on a change in pull request #9634: [TVMScript] Syntax sugar for reads & writes
Posted by GitBox <gi...@apache.org>.
shingjan commented on a change in pull request #9634:
URL: https://github.com/apache/tvm/pull/9634#discussion_r763272130
##########
File path: python/tvm/script/tir/__init__.pyi
##########
@@ -211,8 +211,12 @@ def alloc_buffer(
special_stmt - Reads/Writes
"""
-def reads(read_regions: Union[BufferSlice, List[BufferSlice]]) -> None: ...
-def writes(write_region: Union[BufferSlice, List[BufferSlice]]) -> None: ...
+def reads(
+ read_regions: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
+def writes(
+ write_region: Union[BufferSlice, List[BufferSlice]], *other_regions: BufferSlice
+) -> None: ...
Review comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org