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

[XPU][TritonIntelGPUToLLVM] Do not generate duplicated string constants #3137

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

victor-eds
Copy link
Contributor

Do not generate duplicated string constants as result of tt.assert or tt.print operations. Use a string constant cache to avoid duplication.

Closes #3054.

Do not generate duplicated string constants as result of `tt.assert` or
`tt.print` operations. Use a string constant cache to avoid duplication.

Signed-off-by: victor-eds <[email protected]>
@victor-eds victor-eds requested review from anmyachev, whitneywhtsang and a team January 10, 2025 14:00
@victor-eds victor-eds self-assigned this Jan 10, 2025
@victor-eds victor-eds marked this pull request as draft January 10, 2025 14:03
@victor-eds
Copy link
Contributor Author

victor-eds commented Jan 10, 2025

Fixing this issue would result in overhead in the base case (no tt.assert or tt.print operations used). As we may not have these operations in most cases, I'd rather close the issue as won't fix.

Alternatively, we can merge this PR.

@victor-eds victor-eds marked this pull request as ready for review January 10, 2025 14:39
@anmyachev
Copy link
Contributor

anmyachev commented Jan 10, 2025

Fixing this issue would result in overhead in the base case (no tt.assert or tt.print operations used).

Hi @victor-eds, is the overhead significant? Maybe it is acceptable, considering that we seem to be saving memory?

@victor-eds
Copy link
Contributor Author

Hi @victor-eds, is the overhead significant? Maybe it is acceptable, considering that we seem to be saving memory?

I don't think the overhead is significant, but I wonder if we want to go with any additional overhead to save memory on "debug" scenarios (asserts don't have effect unless TRITON_DEBUG is set and I wouldn't expect prints on release kernels either). I'm fine with this option to save memory when debugging kernels, but:

  • We would improve debug mode performance in exchange of performance mode performance (potentially small overhead, but, still)
  • How many times outside artificial test will we find kernels with several asserts or prints to a point avoiding string duplication is worth it?

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.

Try to avoid duplication among generated assert constants in .llir file
2 participants