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

Support mask analysis against 0, and between two scalars #202

Merged
merged 4 commits into from
Jan 9, 2025

Conversation

kile01
Copy link
Contributor

@kile01 kile01 commented Dec 19, 2024

Support was added for two new scenarios:
arith.cmpi ge %scalar, %c0: aka offset comparison to the lower bound of 0. Mask analysis already has an implicit assumption that the beginning of a mask starts at 0, so support was added to allow this case through and assumes that this comparison evaluates to true.
arith.cmpi slt %scalar,_1 %scalar_2: offset comparison between two scalars. E.g.:

%11 = tt.expand_dims %offset
%cst_4 = arith.constant dense<324> : tensor<16x1xi64>
%23 = arith.cmpi slt, %11, %cst_4 : tensor<16x1xi64>

This example is notable in that we cannot take the normal approach of computing the minimum of the lhs and rhs as the new dimension (the lhs offset may be 0). To handle this, a ternary operator is inserted to evaluate the comparison at runtime. If it succeeds, we keep the existing dimensions from the lhs, otherwise we assume nothing should be loaded/stored.

This change also adds a dump method to both MaskState and PtrState as a small QOL improvement.

@kile01 kile01 changed the title Support Support mask analysis against 0, and between two scalars Dec 19, 2024
@kile01
Copy link
Contributor Author

kile01 commented Dec 19, 2024

@microsoft-github-policy-service agree company="Meta"

@kile01 kile01 marked this pull request as ready for review December 19, 2024 19:43
@nhat-nguyen
Copy link
Collaborator

@kile01 Thank you Kirsten! Sorry for the delay as I went on vacation. Looks like there's a typo in the lit test command. The command we use is:

RUN: triton-shared-opt ... %s | FileCheck %s

@kile01
Copy link
Contributor Author

kile01 commented Jan 2, 2025

triton-shared-opt

No worries, this isn't urgent :) and thank you for the catch!

nhat-nguyen
nhat-nguyen previously approved these changes Jan 8, 2025
Copy link
Collaborator

@nhat-nguyen nhat-nguyen left a comment

Choose a reason for hiding this comment

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

Changes look good to me! This is optional, but would be great if we could also have a small snippet of the triton program that generates the IR (I usually put this as a comment in the lit test)

@kile01
Copy link
Contributor Author

kile01 commented Jan 8, 2025

thanks for the review @nhat-nguyen! I've added the example program as a comment

@kile01
Copy link
Contributor Author

kile01 commented Jan 9, 2025

@nhat-nguyen would you mind doing the honors and merging the PR? I don't seem to have write access

@nhat-nguyen nhat-nguyen enabled auto-merge (squash) January 9, 2025 02:43
@nhat-nguyen nhat-nguyen merged commit 72e73a0 into microsoft:main Jan 9, 2025
3 checks passed
@kile01 kile01 deleted the ptr_analysis branch January 9, 2025 18:35
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.

2 participants