You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "tqchen (via GitHub)" <gi...@apache.org> on 2023/05/29 01:23:45 UTC
[GitHub] [tvm] tqchen opened a new pull request, #14972: [OPENCL] Always use convert_T for type conversion
tqchen opened a new pull request, #14972:
URL: https://github.com/apache/tvm/pull/14972
This PR changes the Cast in OpenCL to always relying on convert_T to get closer to the spec and more reliable.
--
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] tqchen commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209310707
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
i also migrated the original test to the latest TensorIR so the test is future proof.
Interestingly TensorIR becomes smarter :) so the original condition get simplified and becomes `i == 15` here. So I modified the test to ensure we cover most behavior we intend to cover around type casting
--
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] MasterJH5574 merged pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "MasterJH5574 (via GitHub)" <gi...@apache.org>.
MasterJH5574 merged PR #14972:
URL: https://github.com/apache/tvm/pull/14972
--
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] tvm-bot commented on pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tvm-bot (via GitHub)" <gi...@apache.org>.
tvm-bot commented on PR #14972:
URL: https://github.com/apache/tvm/pull/14972#issuecomment-1566355547
<!---bot-comment-->
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](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @echuraev, @elvin-n <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] echuraev commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209333671
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
> so the original condition get simplified and becomes i == 15 here
Do you mean that this condition:
```
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
i % block_size == tvm.tir.const(3, "int32"),
]
),
```
Was transformed to `i == 15`?
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
I took a look at the git history and the original problem which should be covered by this test was that left and right parts of condition should have the same data type. You can see detailed description in #11021. And next this test was added in #11038.
By decreasing the number of conditions in `tvm.tir.all`, you prevent compiler to generate code such as `lcond && rcond`. Am I right? In this case, the original problem won't be tested. Sorry if I missed something.
--
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] echuraev commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209333671
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
> so the original condition get simplified and becomes i == 15 here
Do you mean that this condition:
```
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
i % block_size == tvm.tir.const(3, "int32"),
]
),
```
Was transformed to `i == 15`?
I took a look at the git history and the original problem which should be covered by this test was that left and right parts of condition should have the same data type. You can see detailed description in #11021. And next this test was added in #11038.
By decreasing the number of conditions in `tvm.tir.all`, you prevent compiler to generate code such as `lcond && rcond`. Am I right? In this case, the original problem won't be tested. Sorry if I missed something.
--
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] tqchen commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209415961
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
Sorry I wasn't being clear, right
```
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
i % block_size == tvm.tir.const(3, "int32"),
]
)
```
In TensorIR schedule the ^ condition get transformed to `i == 15`, as indeed that makes sense.
So we might need a different condition to test the chains.
--
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] tqchen commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209535260
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
OK updated to a case where simplification won't happen
--
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] tqchen commented on pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on PR #14972:
URL: https://github.com/apache/tvm/pull/14972#issuecomment-1570914080
@tvm-bot rerun
--
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] echuraev commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1208902841
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
This test has covered a bug in TVM which was fixed. Why did you change the compute function? Can we keep it as it was?
--
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] tqchen commented on a diff in pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "tqchen (via GitHub)" <gi...@apache.org>.
tqchen commented on code in PR #14972:
URL: https://github.com/apache/tvm/pull/14972#discussion_r1209312501
##########
tests/python/unittest/test_target_codegen_opencl.py:
##########
@@ -150,50 +158,38 @@ def check_type_casting(ctx, n, dtype):
tvm.tir.all(
*[
i // block_size == tvm.tir.const(3, "int32"),
- i % block_size == tvm.tir.const(3, "int32"),
Review Comment:
Unfortunately relying on string pattern was too reliable given the set of possible transformations here. So we need to adapt the testcase as we go. If there is a new test cases in TVMScript, i think we can also put it here
--
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] echuraev commented on pull request #14972: [OPENCL] Always use convert_T for type conversion
Posted by "echuraev (via GitHub)" <gi...@apache.org>.
echuraev commented on PR #14972:
URL: https://github.com/apache/tvm/pull/14972#issuecomment-1571382690
@tvm-bot rerun
--
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