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

[frontend] added overflow checks in debug mode #4589

Merged
merged 12 commits into from
Oct 6, 2024
Merged

Conversation

ptillet
Copy link
Collaborator

@ptillet ptillet commented Aug 27, 2024

No description provided.

@@ -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:
Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree

@ptillet ptillet force-pushed the phil/overflow-detect branch from 2bb09c4 to 87660c6 Compare October 6, 2024 03:08
@ptillet ptillet merged commit c54f988 into main Oct 6, 2024
7 checks passed
@ptillet ptillet deleted the phil/overflow-detect branch October 6, 2024 06:09
sfzhu93 pushed a commit to sfzhu93/triton that referenced this pull request Oct 11, 2024
lezcano added a commit to lezcano/triton that referenced this pull request Oct 14, 2024
triton-lang#4589 mistakenly deactivated
these and reverted to the previous always-cast-to-int32 semantics.
lezcano added a commit that referenced this pull request Oct 14, 2024
#4589 mistakenly deactivated
these and reverted to the previous always-cast-to-int32 semantics.
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this pull request Nov 1, 2024
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
rahulsingh-intel pushed a commit to rahulsingh-intel/pytorch that referenced this pull request Nov 5, 2024
…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
Luosuu pushed a commit to Luosuu/triton that referenced this pull request Nov 13, 2024
Luosuu pushed a commit to Luosuu/triton that referenced this pull request Nov 13, 2024
triton-lang#4589 mistakenly deactivated
these and reverted to the previous always-cast-to-int32 semantics.
guacamoleo pushed a commit to guacamoleo/triton that referenced this pull request Nov 14, 2024
guacamoleo pushed a commit to guacamoleo/triton that referenced this pull request Nov 14, 2024
triton-lang#4589 mistakenly deactivated
these and reverted to the previous always-cast-to-int32 semantics.
bertmaher pushed a commit to bertmaher/triton that referenced this pull request Dec 10, 2024
bertmaher pushed a commit to bertmaher/triton that referenced this pull request Dec 10, 2024
triton-lang#4589 mistakenly deactivated
these and reverted to the previous always-cast-to-int32 semantics.
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