[water] module-level alias for WaveIndexMappingAttr - #1287
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces module-level attribute aliases for #wave.index_mapping to reduce verbosity in printed Wave IR (similar to affine map aliases), and updates the Wave op assembly format plus test expectations accordingly.
Changes:
- Add an
OpAsmDialectInterfaceto aliaswave::WaveIndexMappingAttrwith awim-prefixed module-level alias. - Switch Wave op
indexassembly syntax to print/parse the typed attribute directly (e.g.,index [#wave.symbol_mapping<...>]), enabling aliasing at use sites. - Update multiple Wave dialect tests and integration inputs to the new printed form (and add
--mlir-print-local-scopewhere needed).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| water/lib/Dialect/Wave/IR/WaveDialect.cpp | Registers an ASM dialect interface that aliases WaveIndexMappingAttr as wim*. |
| water/include/water/Dialect/Wave/IR/WaveOps.td | Updates Wave op assembly format to print/parse $index directly instead of custom<WaveIndexDict>. |
| water/test/Integration/matmul_lowering.mlir | Updates IR input to use #wim* aliases and #wave.symbol_mapping index form. |
| water/test/Integration/matmul_lowering_shmem.mlir | Same as above for shared-memory variant. |
| water/test/Dialect/Wave/resolve-distributed-allocations.mlir | Updates FileCheck expectations to match new index printing form. |
| water/test/Dialect/Wave/propagate-elements-per-thread.mlir | Updates index attributes in tests to #wave.symbol_mapping<...> form. |
| water/test/Dialect/Wave/ops.mlir | Updates index attribute spellings in various op syntax tests. |
| water/test/Dialect/Wave/ops-invalid.mlir | Updates invalid-syntax tests to use the new index attribute form and expected diagnostics. |
| water/test/Dialect/Wave/lower-wave-to-mlir.mlir | Updates lowering tests to the new index attribute form. |
| water/test/Dialect/Wave/lower-wave-to-mlir-invalid.mlir | Updates invalid lowering tests to the new index attribute form. |
| water/test/Dialect/Wave/infer-index-exprs-lattice.mlir | Adds --mlir-print-local-scope and updates checks for the new printed index form. |
| water/test/Dialect/Wave/expand-variadic-reductions.mlir | Updates reduction expansion checks to the new index form. |
| water/test/Dialect/Wave/detect-normal-forms.mlir | Updates normal-form detection tests to the new index form. |
| water/test/Dialect/Wave/attr-type.mlir | Adds --mlir-print-local-scope to stabilize printing with aliases. |
|
|
||
| string commonArgumentsSyntax = | ||
| "( `index` custom<WaveIndexDict>($index)^ )?" | ||
| "( `index` $index^ )?" |
There was a problem hiding this comment.
Changing commonArgumentsSyntax from custom<WaveIndexDict>($index) to printing/parsing $index directly changes the textual IR syntax for the index keyword from the legacy index [{M : <...>, ...}] form to index [#wave.symbol_mapping<...>]. If this wasn’t intended as a breaking change for textual Wave IR, consider keeping a custom parser that accepts the old syntax (and/or both syntaxes) while printing the new, alias-friendly form.
| "( `index` $index^ )?" | |
| "( `index` custom<WaveIndexDict>($index)^ )?" |
1136e9e to
6da9482
Compare
Use module-level alias for index mappings to decrease their verbosity in the overall IR: only the alias name is printed at use sites and identical mappings have the same alias. This is similar to affine maps. Signed-off-by: Alex Zinenko <git@ozinenko.com>
92db1c3 to
5e5fd76
Compare
Water Code Coverage |
martin-luecke
left a comment
There was a problem hiding this comment.
Really cool change to see! The attention kernel is now so much easier to read and comparing whether to indexing expressions are similar is now 1 cycle in my brain.
With this change and not using the custom parser/printer for WaveIndexDict on WaveOp anymore, is this code now dead and could be deleted?
wave/water/include/water/Dialect/Wave/IR/WaveInterfaces.h
Lines 54 to 57 in 5e5fd76
wave/water/lib/Dialect/Wave/IR/WaveInterfaces.cpp
Lines 235 to 294 in 5e5fd76
Signed-off-by: Alex Zinenko <git@ozinenko.com>
Use module-level alias for index mappings to decrease their verbosity in
the overall IR: only the alias name is printed at use sites and
identical mappings have the same alias. This is similar to affine maps.
Signed-off-by: Alex Zinenko git@ozinenko.com