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 2023/01/05 21:19:51 UTC
[GitHub] [tvm] Lunderberg opened a new pull request, #13708: [Arith] Simplify to positive numerators in floordiv/floormod
Lunderberg opened a new pull request, #13708:
URL: https://github.com/apache/tvm/pull/13708
Negative numerators to modulo/remainder operations are not supported by the Vulkan API. While the SPIR-V instructions
[`OpSRem`](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpSRem) and [`OpSMod`](https://registry.khronos.org/SPIR-V/specs/unified1/SPIRV.html#OpSMod) have identical semantics to `tir::Mod` and `tir::FloorMod`, respectively, use of either instruction within Vulkan results in undefined behavior. From the [Vulkan spec](https://registry.khronos.org/vulkan/specs/1.3/html/chap37.html#spirvenv-op-prec):
> For the OpSRem and OpSMod instructions, if either operand is negative the result is undefined.
>
> Note: While the OpSRem and OpSMod instructions are supported by the Vulkan environment, they require non-negative values and thus do not enable additional functionality beyond what OpUMod provides.
This issue was first noticed in https://github.com/apache/tvm/pull/13530, where use of integer arithmetic resulted in negative numerators. This hadn't caused issues previously, because most use of div/mod use a denominator that is a power of two. In these cases, `tir.LowerIntrin` implements floordiv and floormod using only bitwise operations. When the denominator isn't a power of two, both `tir::FloorDiv` and `tir::FloorMod` are implemented in terms of `tir::Mod`, which triggers the undefined behavior for negative numerators.
This commit implements additional simplification rules that preferentially removes negative values from the numerators. For example, simplifying `floormod(i - 2, 8)` to `floormod(i + 6, 8)`, and simplifying `floordiv(i - 2, 8)` to `floordiv(i + 6, 8) - 1`. These handle the most common case, where some index variable is being offset by a negative constant.
--
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] Lunderberg closed pull request #13708: [Arith] Simplify to positive numerators in floordiv/floormod
Posted by GitBox <gi...@apache.org>.
Lunderberg closed pull request #13708: [Arith] Simplify to positive numerators in floordiv/floormod
URL: https://github.com/apache/tvm/pull/13708
--
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] Lunderberg commented on pull request #13708: [Arith] Simplify to positive numerators in floordiv/floormod
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13708:
URL: https://github.com/apache/tvm/pull/13708#issuecomment-1374370669
The latest change to the simplification caused more breakages than I think it is worth. Closing this PR in favor of https://github.com/apache/tvm/pull/13724, which has a more limited scope.
--
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] tvm-bot commented on pull request #13708: [Arith] Simplify to positive numerators in floordiv/floormod
Posted by GitBox <gi...@apache.org>.
tvm-bot commented on PR #13708:
URL: https://github.com/apache/tvm/pull/13708#issuecomment-1372787541
<!---bot-comment-->
Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
<!--bot-comment-ccs-start-->
* cc @junrushao <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
<sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>
--
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] Lunderberg commented on pull request #13708: [Arith] Simplify to positive numerators in floordiv/floormod
Posted by GitBox <gi...@apache.org>.
Lunderberg commented on PR #13708:
URL: https://github.com/apache/tvm/pull/13708#issuecomment-1372792682
When implementing this change, I had also considered making a Vulkan-specific pass, rather than changing the simplification. However, the effect of such a pass could be undone by a later simplification, so I believe this to be the better solution.
--
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