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/10/20 21:10:21 UTC
[GitHub] [tvm] electriclilies opened a new pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
electriclilies opened a new pull request #9334:
URL: https://github.com/apache/tvm/pull/9334
Some non computational ops have the FTVMCompute attr set to the TOPI identity function. Since the ops are noncomputational, this compute function should do nothing. In this PR, I remove the FTVMCompute attribute from all noncomputational Relay ops.
--
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] electriclilies commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948806027
@masahi 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] masahi edited a comment on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948921743
> For removing FInferCorrectLayout, it looks like most of attrs are set to ElemwiseArbitraryLayout, but some of the QNN ops are set to custom layout methods
Oh yes, `InferLayout` functions for QNN ops are used when a user calls `ConvertLayout` on a quantized model after the model is imported. So this is very important. `QnnConcatenateLayout` is indeed used and there are other QNN ops that have a specialized `FInferCorrectLayout`, such as `RequantizeInferCorrectLayout`.
On the other hand, the ops you removed `FTVMCompute` from in this PR are not "user-facing" in the same sense that QNN ops are, they are only used by VM during compilation and `ConvertLayout` pass will never see them. So it should be safe to remove `FInferCorrectLayout` from them.
I didn't realize that QNN ops are marked `TNonComputational`, but they are kind of distinct from other `TNonComputational` ops in this sense.
--
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] electriclilies commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948086431
@AndrewZhaoLuo Since they are marked non computational the compute shouldn't ever be running. If the compute is running then they shouldn't be marked as non computational.
I think that FTVMCompute ended up on some of these ops because people were just copy pasting from other ops.
--
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] masahi merged pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #9334:
URL: https://github.com/apache/tvm/pull/9334
--
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] AndrewZhaoLuo commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948056109
Can you go into a little more detail on why these have FTVMCompute? My worry is that there is some pass somewhere which grabs this attribute for these and this change will break things.
--
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] electriclilies edited a comment on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies edited a comment on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948086431
@AndrewZhaoLuo Since they are marked non computational the compute shouldn't ever be running. If the compute is running then they shouldn't be marked as non computational.
I think that FTVMCompute ended up on some of these ops because people were just copy pasting from other ops, I don't think there is any "purpose" for the FTVMCompute attr in these cases.
--
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] AndrewZhaoLuo commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
AndrewZhaoLuo commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948177072
Makes sense, I just wanted an audit to make sure people aren't using the FTVMCompute attr willy nilly through the code base. It doesn't look like they are though.
--
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] electriclilies commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948834590
@masahi For removing FInferCorrectLayout, it looks like most of attrs are set to ElemwiseArbitraryLayout, but some of the QNN ops are set to custom layout methods (QNN concatenate has the method QnnConcatenateLayout). Do you know if that layout method is actually used?
--
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] masahi commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948921743
> For removing FInferCorrectLayout, it looks like most of attrs are set to ElemwiseArbitraryLayout, but some of the QNN ops are set to custom layout methods
Oh yes, `InferLayout` functions for QNN ops are used when a user calls `ConvertLayout` on a quantized model after the model is imported. So this is very important.
On the other hand, the ops you removed `FTVMCompute` from in this PR are not "user-facing" in the same sense that QNN ops are, they are only used by VM during compilation and `ConvertLayout` pass will never see them. So it should be safe to remove `FInferCorrectLayout` from them.
I didn't realize that QNN ops are marked `TNonComputational`, but they kind of distinct from other `TNonComputational` ops in this sense.
--
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] electriclilies commented on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies commented on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-949003599
@masahi Yeah I think I actually added the TNonComputational option to the QNN ops-- conceptually I guess they are more "symbolic" than non-computational, since they do conceptually compute something, but that compute is injected during legalization, which is wayyy before lowering.
I added it because when I was working on quantization, one time I accidentally skipped the legalize pass and then lowering broke in a weird way. But with the TNoncomputational attr set it would just throw an 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] electriclilies edited a comment on pull request #9334: [Relay] Remove FTVMCompute from TNonComputational ops
Posted by GitBox <gi...@apache.org>.
electriclilies edited a comment on pull request #9334:
URL: https://github.com/apache/tvm/pull/9334#issuecomment-948834590
@masahi For removing FInferCorrectLayout, it looks like most of attrs are set to ElemwiseArbitraryLayout, but some of the QNN ops are set to custom layout methods (QNN concatenate has the method QnnConcatenateLayout). Do you know if that layout method is actually used?
(cc @AndrewZhaoLuo, you might be interested in this question :) )
--
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