use gemm template in the water analysis test#833
use gemm template in the water analysis test#833ftynse wants to merge 2 commits intousers/ftynse/infer-exprs-wave-constraintsfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the water analysis test to use the generic GEMM template instead of a local duplicate implementation. Previously, the test maintained its own GEMM kernel definition because the template lacked support for wave constraints and used unsafe unceiled division. With recent improvements addressing both issues, the local implementation can now be removed in favor of the shared template.
Changes:
- Removed local
_get_gemm_kernel()function and replaced it with importedget_gemm_kernel()from the GEMM template - Updated hyperparameter handling to filter only needed symbols and conditionally override shared memory settings
- Cleaned up unused imports
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| hyperparams = { | ||
| s: v | ||
| for s, v in hyperparams.items() | ||
| if s | ||
| in [ | ||
| tkl.sym.M, | ||
| tkl.sym.N, | ||
| tkl.sym.K, | ||
| tkl.sym.BLOCK_M, | ||
| tkl.sym.BLOCK_N, | ||
| tkl.sym.BLOCK_K, | ||
| tkl.sym.ADDRESS_SPACE, | ||
| ] | ||
| } |
There was a problem hiding this comment.
Consider extracting the list of required symbols to a constant or variable before the loop to improve readability and avoid recreating the list on each test iteration. This would make the filtering logic clearer and more maintainable.
97085d5 to
e707305
Compare
We previously couldn't use it due to missing support for wave constraints and it using (unsafe) unceiled division in them. Both are now available with safe workarounds. Signed-off-by: Alex Zinenko <git@ozinenko.com>
Signed-off-by: Alex Zinenko <git@ozinenko.com>
5fab8ea to
6f4ccc3
Compare
We previously couldn't use it due to missing support for wave constraints and it using (unsafe) unceiled division in them. Both are now available with safe workarounds.