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 2022/06/22 13:24:32 UTC
[GitHub] [tvm] wrongtest opened a new pull request, #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
wrongtest opened a new pull request, #11828:
URL: https://github.com/apache/tvm/pull/11828
Fix a bug for let support, it should not put non-index typed values into `dom_map`.
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1165059549
Looks like some situations happened in the CI...
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1165939612
@wrongtest would you mind retriggering the CI? looks like it's down
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1164035082
Agree that we shouldn’t bind vectorized data types. Besides that, is that anything else we shouldn’t bind? If not, we can instead only check lanes field
--
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] wrongtest commented on pull request #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
wrongtest commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1163651679
> Hey thanks for the contribution! I don't think I fully comprehend this change (and the unittest) so would love to hear more about your insights :-) With this change, whether or not to add a `tir::Var` into `dom_map` is based on its dtype. Why is it the case?
Emmmmm，my understand is that since the pass is only interested in buffer indices，record index typed vars could be safe. While arith utilities are written to handle integers，binding things like SinglePoint(int32x8) would just fail on sanity checks of dom analysis (eg, EvalNDSet).
--
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] wrongtest commented on pull request #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
wrongtest commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1164297106
> Perhaps we can add an assertion in the Bind method where we always assume the value shouldn’t be vectorized
Thanks for the notes~ I'd like to add one ICHECK line to see if there are broken cases. For vectors, however, I'm not sure it should never be considered to get analyzed/simplified. For `CompactBufferAllocation`, just integer analysis part (IntSets) should not work with vectors.
--
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] wrongtest commented on pull request #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
wrongtest commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1164292056
> is that anything else we shouldn’t bind
And kVoid and kHandle should not bind.
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1167964142
Retriggering as CI is back to life
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1164035633
Perhaps we can add an assertion in the Bind method where we always assume the value shouldn’t be vectorized
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1164766003
@wrongtest Thank you for your detailed explanation! That makes a lot of sense to me :-)
--
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 #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on PR #11828:
URL: https://github.com/apache/tvm/pull/11828#issuecomment-1163415778
Hey thanks for the contribution! I don't think I fully comprehend this change (and the unittest) so would love to hear more about your insights :-) With this change, whether or not to add a `tir::Var` into `dom_map` is based on its dtype. Why is it the 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.
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 merged pull request #11828: [TIR][BugFix] Do not bind non-index type value of lets in CompactBufferAllocation
Posted by GitBox <gi...@apache.org>.
junrushao1994 merged PR #11828:
URL: https://github.com/apache/tvm/pull/11828
--
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