[water] replace ReadWriteBoundsAttr with SymbolMappingAttr, NFC#930
Merged
[water] replace ReadWriteBoundsAttr with SymbolMappingAttr, NFC#930
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR replaces WaveReadWriteBoundsAttr with WaveSymbolMappingAttr to simplify access to bounds attributes and use context-uniqued WaveSymbolAttr instead of raw strings for dictionary keys.
Changes:
- Renamed
WaveReadWriteBoundsAttrtoWaveSymbolMappingAttrthroughout the codebase - Changed attribute storage from dictionary-based (string keys) to parallel arrays of symbol attributes and expression lists
- Updated Python bindings to support dict-like access with
__getitem__,__len__, and__contains__methods
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| wave_lang/kernel/wave/mlir_converter/water_emitter.py | Updated attribute creation to use WaveSymbolMappingAttr |
| wave_lang/kernel/wave/mlir_converter/fx_emitter.py | Updated import and conversion function to iterate over new attribute structure |
| water/test/Dialect/Wave/python_bindings.py | Updated tests to use new attribute name and added tests for dict-like access methods |
| water/test/Dialect/Wave/ops.mlir | Updated MLIR test syntax from read_write_bounds to symbol_mapping with @ symbol prefix |
| water/test/Dialect/Wave/ops-invalid.mlir | Updated error test cases to use new attribute syntax and removed obsolete test |
| water/test/Dialect/Wave/lower-wave-to-mlir.mlir | Updated MLIR lowering tests to use new attribute syntax |
| water/python/WaterExtensionNanobind.cpp | Rewrote Python bindings to use parallel arrays instead of dictionary, added dict-like access methods |
| water/lib/Dialect/Wave/Transforms/LowerReadWriteOps.cpp | Updated to use lookup method on new attribute structure |
| water/lib/Dialect/Wave/IR/WaveOps.cpp | Simplified verification logic to work with symbol attributes instead of strings |
| water/lib/Dialect/Wave/IR/WaveAttrs.cpp | Added builder method for WaveSymbolMappingAttr |
| water/lib/CAPI/Dialects.cpp | Rewrote C API functions to use parallel arrays and provide lookup functionality |
| water/include/water/c/Dialects.h | Updated C API declarations with new function signatures |
| water/include/water/Dialect/Wave/IR/WaveOps.td | Changed attribute type references in operation definitions |
| water/include/water/Dialect/Wave/IR/WaveAttrs.td | Removed WaveReadWriteBoundsAttr definition and enhanced WaveSymbolMappingAttr |
| lit_tests/kernel/wave/mlir_converter.py | Updated test expectations for new attribute syntax |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
martin-luecke
approved these changes
Feb 20, 2026
Contributor
martin-luecke
left a comment
martin-luecke
left a comment
There was a problem hiding this comment.
adding a few more nits
This removes one level of indirection when accessing the bounds attribute and another level of indirection in using context-uniqued WaveSymbolAttr instead of raw strings. Signed-off-by: Alex Zinenko <git@ozinenko.com>
- fix naming nits - add support for string inclusion checks in py bindings - avoid exceptions in py bindings on happy path - beef up py bindings tests Signed-off-by: Alex Zinenko <git@ozinenko.com>
ftynse
force-pushed
the
users/ftynse/replace-bounds
branch
from
3735e83 to
7abf646
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This removes one level of indirection when accessing the bounds attribute and another level of indirection in using context-uniqued WaveSymbolAttr instead of raw strings.
Partially supersedes #730