Skip to content

Comments

[water] introduce WaveSymbolMappingAttr#929

Merged
ftynse merged 2 commits intomainfrom
users/ftynse/symbol-mapping-attr
Feb 20, 2026
Merged

[water] introduce WaveSymbolMappingAttr#929
ftynse merged 2 commits intomainfrom
users/ftynse/symbol-mapping-attr

Conversation

@ftynse
Copy link
Contributor

@ftynse ftynse commented Feb 19, 2026

This is a dedicated attribute usable as a nearly drop-in replacement for a dictionary attribute of expression lists. It uses WaveSymbolAttr instances as keys instead of strings and by design preserves the order in which keys appear.

Usage of this attribute will be introduced progressively in separate commits.

Partially supersedes #730

This is a dedicated attribute usable as a nearly drop-in replacement for
a dictionary attribute of expression lists. It uses `WaveSymbolAttr`
instances as keys instead of strings and by design preserves the order
in which keys appear.

Usage of this attribute will be introduced progressively in separate
commits.

Signed-off-by: Alex Zinenko <git@ozinenko.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Introduces WaveSymbolMappingAttr, a new ordered mapping attribute for the Wave dialect that maps WaveSymbolAttr keys to WaveExprListAttr values while preserving insertion order (intended as an ordered alternative to dictionary-based attributes). It also adds a small test-only op and MLIR tests to validate printing/parsing, ordering, and verifier behavior.

Changes:

  • Add WaveSymbolMappingAttr definition (TableGen) plus custom parse/print, verification, and helper APIs (verifyResultCount, lookup).
  • Add water_test.wave_symbol_mapping test op with a verifier that checks a uniform result count across mapped expr lists.
  • Add positive/negative MLIR tests for parsing/printing, ordering preservation, duplicate keys, and result-count verification.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
water/include/water/Dialect/Wave/IR/WaveAttrs.td Declares WaveSymbolMappingAttr and its public helper methods.
water/lib/Dialect/Wave/IR/WaveAttrs.cpp Implements custom assembly format parsing/printing and verification for the new attribute.
water/test/lib/Dialect/WaterTestDialect.td Adds a dummy op to hold/verify WaveSymbolMappingAttr in tests.
water/test/lib/Dialect/WaterTestDialect.cpp Implements the dummy op verifier using verifyResultCount.
water/test/Dialect/Wave/attr-type.mlir Adds roundtrip/ordering tests for #wave.symbol_mapping.
water/test/Dialect/Wave/attr-type-invalid.mlir Adds negative tests for duplicate keys and mismatched expr_list result counts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

@martin-luecke martin-luecke left a comment

Choose a reason for hiding this comment

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

Nothing really to add beyond what copilot wrote.
IMO this could have been combined with #928

Signed-off-by: Alex Zinenko <git@ozinenko.com>
@ftynse
Copy link
Contributor Author

ftynse commented Feb 20, 2026

IMO this could have been combined with #928

#930 is based on this but does not require #928, which allows for better staging if generalization ends up being problematic / needing deeper changes to address review

@ftynse
Copy link
Contributor Author

ftynse commented Feb 20, 2026

Numeric issue is irreleavnt

@ftynse ftynse merged commit d067bf2 into main Feb 20, 2026
14 of 15 checks passed
@ftynse ftynse deleted the users/ftynse/symbol-mapping-attr branch February 20, 2026 12:45
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