Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ARITH] Refactor to use explicit div/mod functions instead of operators. #4000

Merged
merged 2 commits into from
Sep 25, 2019

Conversation

tqchen
Copy link
Member

@tqchen tqchen commented Sep 25, 2019

This PR is part of #3977

Summary of changes:

  • Removed the operator overload for % and / and explicitly ask developers to use the specific division functions
  • Introduce indexdiv and indexmod, a special division function can be redirected to either floordiv/truncdiv. The requirement to use this function is that we know the operands are non-negative, we might introduce additional constraints attachment if necessary as in https://discuss.tvm.ai/t/discuss-embed-more-bound-information-into-var-or-expr/4079
  • Refactor the existing code to make use of the division functions
  • For now we maintain the old behavior of the code, and we can switch the implementation of indexdiv/mod in a separate PR.

@tqchen
Copy link
Member Author

tqchen commented Sep 25, 2019

cc @yzhliu @icemelon9 @wweic @xqdan @sgrechanik-h @derisavi @umangyadav @icemelon9 @kazum

@tqchen tqchen changed the title [ARITH] Use explicit div/mod functions instead of operators. [ARITH] Refactor use explicit div/mod functions instead of operators. Sep 25, 2019
@tqchen tqchen changed the title [ARITH] Refactor use explicit div/mod functions instead of operators. [ARITH] Refactor to use explicit div/mod functions instead of operators. Sep 25, 2019
@ajtulloch
Copy link
Contributor

This looks really great @tqchen.

@tqchen
Copy link
Member Author

tqchen commented Sep 25, 2019

Additional notes on division modes:

  • floordiv is relatively easier for simplification as it does not require to know the sign of operand a
    • we can do floordiv(a+ 17, 16) -> floordiv(a + 1, 16), regardless of the sign of a
    • However, we need to prove the sign of the operands eventually when lower to the low level code, as we can only lower floordiv(a, b)->truncdiv(a, b) if a>=0 and b>=0
  • truncdiv is easier to for codegen, as it directly corresponds to low level instructions, but it is harder for simplification:
    • truncdiv(a+ 17, 16) -> truncdiv(a + 1, 16) only if a+1>=0.

So we have to make the choice of division mode for index calculations. indexdiv/mod tries to isolate that choice into a single place. It also brings additional information to the function that a and b are non-negative at the moment we call the function.

  • Currently, we lower indexdiv-> truncdiv, we plan to move to floordiv
  • We can plugin additional hints: indexdiv(a, b)->floordiv(a, nonneg_hint(b) if b is Var else b)

Copy link
Member

@icemelon icemelon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tqchen tqchen merged commit f0079a5 into apache:master Sep 25, 2019
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
…rs. (apache#4000)

* [ARITH] Use explicit div/mod functions instead of operators.

* fix pooling case
wweic pushed a commit to wweic/tvm that referenced this pull request Sep 30, 2019
…rs. (apache#4000)

* [ARITH] Use explicit div/mod functions instead of operators.

* fix pooling case
wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 1, 2019
…rs. (apache#4000)

* [ARITH] Use explicit div/mod functions instead of operators.

* fix pooling case
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants