-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
[frontend] added overflow checks in debug
mode
#4589
Conversation
python/triton/language/semantic.py
Outdated
@@ -128,6 +128,26 @@ def binary_op_type_checking_impl(lhs: tl.tensor, rhs: tl.tensor, builder: ir.bui | |||
return lhs, rhs | |||
|
|||
|
|||
def binary_op_sanitize_overflow_impl(lhs: tl.tensor, rhs: tl.tensor, builder: ir.builder, binary_op: callable): | |||
if lhs.type.scalar.int_bitwidth >= 64 or not builder.options.debug: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be a separate option from enabling asserts generally, so it's possible to enable asserts without running overflow checks in every kernel? I'm thinking specifically of inductor which uses asserts to check indirect indexing expressions, but wouldn't want the performance hit of checking every index calculation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree
2bb09c4
to
87660c6
Compare
triton-lang#4589 mistakenly deactivated these and reverted to the previous always-cast-to-int32 semantics.
#4589 mistakenly deactivated these and reverted to the previous always-cast-to-int32 semantics.
Reported in https://github.com/triton-lang/triton/pull/5033/files#r1825438371 This was broken by #4589
Reported in https://github.com/triton-lang/triton/pull/5033/files#r1825438371 This was broken by #4589
In upstream triton, triton-lang/triton#4589 introduces overflow checks. However, overflow checks likely add some overhead, and have some correctness bugs at the moment (e.g. triton-lang/triton#5033). Let's set `sanitize_overflow=False` but keep `debug=True` so that we can keep using device_assert but without the additional asserts added by `sanitize_overflow`. Pull Request resolved: #139502 Approved by: https://github.com/bertmaher
…9502) In upstream triton, triton-lang/triton#4589 introduces overflow checks. However, overflow checks likely add some overhead, and have some correctness bugs at the moment (e.g. triton-lang/triton#5033). Let's set `sanitize_overflow=False` but keep `debug=True` so that we can keep using device_assert but without the additional asserts added by `sanitize_overflow`. Pull Request resolved: pytorch#139502 Approved by: https://github.com/bertmaher
triton-lang#4589 mistakenly deactivated these and reverted to the previous always-cast-to-int32 semantics.
triton-lang#4589 mistakenly deactivated these and reverted to the previous always-cast-to-int32 semantics.
triton-lang#4589 mistakenly deactivated these and reverted to the previous always-cast-to-int32 semantics.
No description provided.