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/04/19 08:17:21 UTC
[GitHub] [tvm] llehtahw opened a new pull request #7879: [Tensorize]Fix tensorize error while reusignbai
llehtahw opened a new pull request #7879:
URL: https://github.com/apache/tvm/pull/7879
After
```
Check failed: (expr_equal(lhs, rhs)) is false: Failed to match the compute with TensorIntrin tensor_intrin's declaration provided= (a[i] + b[i]), intrin= (a[i] + b[i])
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] llehtahw commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
llehtahw commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823957245
> why not use call_extern, etc. to achieve the same goal?
In fact, my example was just drafted for showing the `Check failed`, so is't not important whether to use `call_extern` or `call_packed` here.
Actually I use `tensorize` to take over the whole compute, with compute body generated else where, so `tensorize` is the only thing I do in such a schedule. I reused the compute body by accident but it worked for long time, and cost a lot to debug 😆
> I think. Anyway, I think it is better to fix this issue by updating LHS also.
+1
Thanks for you explaining @leeexyz
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] llehtahw closed pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
llehtahw closed pull request #7879:
URL: https://github.com/apache/tvm/pull/7879
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] llehtahw commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
llehtahw commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-822935786
> This example seems not right to me. At the first glance, I don't think we should reuse the compute in this case.
I agree with you. Not only this example, we are able to avoid reusing in any case.
But basically, reusing is never a special case for expr analyzing or tensorize matching. Should TVM complains about the that usage?
If reusing should be avoided, I think we still need some clearer error message to inform the user to correct the wrong code.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
leeexyz commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823752535
@llehtahw @comaniac The modification does not consider this reusing case. :( Substitute LHS (aka the provide) should solve this reusing case. The reason is after Normalize step, IterVar i is rebased, but only RHS (aka the intrin) has been updated.
```c++
// src/te/operation/tensorize.cc:330
PrimExpr lhs = ana.Simplify(Substitute(body[i], value_map));
```
I think it is okay to reuse compute since this is a way to reuse the compute concepts to describe the behavior of HW intrinsic. Actually, we also do some reusings, but we don't use the same compute directly, what we do like follows.
```
a, b, c = get_compute_args()
s = tvm.te.create_schedule([c.op])
# just like create a new op
_, _, intrin_c = get_compute_args()
# get the intrinsic
intrin_c = get_intrin(intrin_c)
# do tensorize
s[c].tensorize(c.op.axis[0], intrin_c)
tvm.lower(s, (a, b, c))
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] llehtahw commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
llehtahw commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-822403781
@leeexyz @comaniac do you have any suggestion?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
leeexyz commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823822447
What I need to explain more is in our case, we use compute to define the HW intrinsic, it is may as the same as Operators, such as element-wise. But it is not the same compute (I mean the same addresses of all nodes), instead of the concept. :)
For example, an Add op has 4 dims (i0, i1, i2, i3) and the three innermost (i1, i2, i3) axes will be split, then after reorder, the region of the inner axes (i1.i, i2.i, i3.i) will be tensorized to an Intrin_Add. In this situation, Add op and the Intrin_Add share the compute.
So, in the reusing case, it is more natural to `call_extern`, I think. Anyway, I think it is better to fix this issue by updating LHS also.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz removed a comment on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
leeexyz removed a comment on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823809171
@llehtahw Thanks for pointing this out. :) Let me update it and add a new case.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
leeexyz commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823809171
@llehtahw Thanks for pointing this out. :) Let me update it and add a new case.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
leeexyz commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823814290
@llehtahw Thanks for pointing this out. :) One more thing, why not use call_extern, etc. to achieve the same goal?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] llehtahw commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
llehtahw commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-823773672
Thank you @leeexyz .
```
_, _, intrin_c = get_compute_args()
```
That's also my workaround.
I think both of the two usage should be considered valid. Would you like to create another PR to complete #7497?
And this will be closed.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] comaniac commented on pull request #7879: [Tensorize]Fix tensorize error while reusing compute
Posted by GitBox <gi...@apache.org>.
comaniac commented on pull request #7879:
URL: https://github.com/apache/tvm/pull/7879#issuecomment-822614731
This example seems not right to me. At the first glance, I don't think we should reuse the compute in this case.
@leeexyz @tqchen please share your thoughts.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org