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/08/13 08:23:48 UTC

[GitHub] [tvm] Johnson9009 commented on pull request #8706: [TIR] enhance tir signed-unsigned cast

Johnson9009 commented on pull request #8706:
URL: https://github.com/apache/tvm/pull/8706#issuecomment-898280034


   @ganler @YuchenJin @tqchen I happens to file a PR relevant to this change, after reading all of your preceding discussing, I think there maybe have something not clear.
   
   From reading the PR, I found, for the test case here `tir.const(1, "int32").__eq__(tir.const(1, "uint1"))`, the old behavior of BinaryOpMatchTypes is `int32 == uint1  -> int32 == int32(uint1)`, it is exactly same with the job that this PR does.
   
   Then I feel it is strange why this PR and my own PR can make `assert tir.const(1) == tir.const(True)` work, after debug it step-by-step I found the reason is the function we use "cast" instead the "tir::Cast" used by "SimpleCast".
   
   https://github.com/apache/tvm/blob/a06863ac9406c027b29f346fb6177268f612912d/src/tir/op/op.cc#L273-L284
   
   We can see it is the logic around line 279 that eliminate the CastNode, so the function `cast` is do some constant fold optimization.
   
   > Hi @ganler, Overall LGTM! Thanks for elaborating on the issue and implementing the fix!
   > 
   > One question is that whether we want to also make `assert tir.const(1, "uint2") == tir.const(1, "int2")` work? I think since `lhs` and `rhs` have the same bits, `SimpleCast` will be applied to both sides, but the same error throws.
   
   @YuchenJin For the scenario that unsigned and signed integer have the same width, I think we should consistent with C standard (e.g., "int32 + uint32" -> "uint32(int32) + uint32") instead of current logic "int32 + uint32" -> "int32 + int32(uint32)", please have a look of my PR #8733, we implement a new AI chip backend of TVM, now we meet this issue, we need make a pass to change the implicit conversion consistent with C standard. 
   


-- 
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