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 2020/06/25 04:54:06 UTC

[GitHub] [incubator-tvm] yzhliu opened a new pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

yzhliu opened a new pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922


   @tqchen @sergei-grechanik please take a look.
   


----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 edited a comment on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 edited a comment on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649639015


   @tqchen, @yzhliu : I have one concern here(However not related to this PR).
   As you see the expr below:
   expr = (((x0 - floordiv((0 - (x1*5)), 2)) - 1) + floordiv((37 + ((x1 + 7)*-5)), 2))
   
   should be evaluated to actually
   expr = x0
   
   but with current simplify stage(rewrite_simplify + canonical_simplify), it evaluates to below:
   expr = ((floordiv((0 - (x1: int32*5)), 2) + x0: int32) - floordiv((x1*-5), 2))
   
   But when i run stages as (rewrite_simplify + canonical_simplify + rewrite_simplify) it evaluates perfectly to x0.
   
   So may be we need to run these stages with some customized config every time or until there are no more changes?


----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649639015


   @tqchen, @yzhliu : I have one concern here.
   As you see the expr below:
   expr = (((x0 - floordiv((0 - (x1*5)), 2)) - 1) + floordiv((37 + ((x1 + 7)*-5)), 2))
   
   should be evaluated to actually
   expr = x0
   
   but with current simplify stage(rewrite_simplify + canonical_simplify), it evaluates to below:
   expr = ((floordiv((0 - (x1: int32*5)), 2) + x0: int32) - floordiv((x1*-5), 2))
   
   But when i run stages as (rewrite_simplify + canonical_simplify + rewrite_simplify) it evaluates perfectly to x0.
   
   So may be we need to run these stages with some customized config every time or until there are no more changes?


----------------------------------------------------------------
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] [incubator-tvm] yzhliu commented on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
yzhliu commented on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649705664


   @ANSHUMAN87 good catch, this is what the argument `step` for I added in #5618


----------------------------------------------------------------
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] [incubator-tvm] tqchen commented on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649726080


   @yzhliu please also send a patch to v0.6


----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 commented on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 commented on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649636006


   @yzhliu :  The issue is fixed indeed, Thanks!
    I think it would be good if we add the issue test case as well.
   
   --> expr = (((x0 - floordiv((0 - (x1*5)), 2)) - 1) + floordiv((37 + ((x1 + 7)*-5)), 2))
   
   


----------------------------------------------------------------
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] [incubator-tvm] yzhliu merged pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
yzhliu merged pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922


   


----------------------------------------------------------------
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] [incubator-tvm] tqchen commented on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649615620


   Thanks @yzhliu , seems we need a different testcase, that uses a truncdiv but still triggers DivModeCompatibleTo


----------------------------------------------------------------
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] [incubator-tvm] ANSHUMAN87 edited a comment on pull request #5922: [BACKPORT-0.6][Bugfix][Arith] keep div_mode during floordiv simplify

Posted by GitBox <gi...@apache.org>.
ANSHUMAN87 edited a comment on pull request #5922:
URL: https://github.com/apache/incubator-tvm/pull/5922#issuecomment-649636006


   @yzhliu :  The original issue is fixed indeed, Thanks!
    I think it would be good if we add the issue test case as well.
   
   --> expr = (((x0 - floordiv((0 - (x1*5)), 2)) - 1) + floordiv((37 + ((x1 + 7)*-5)), 2))
   
   


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