Skip to content

[FIRRTL][OM] Add string.concat canonicalization#9997

Merged
seldridge merged 6 commits intomainfrom
dev/seldridge/string-concat-canonicalize
Mar 23, 2026
Merged

[FIRRTL][OM] Add string.concat canonicalization#9997
seldridge merged 6 commits intomainfrom
dev/seldridge/string-concat-canonicalize

Conversation

@seldridge
Copy link
Member

Add canonicalization patterns for firrtl.string.concat and
om.string.concat that:

  1. flatten nested string.concat operations,
  2. merge consecutive constant strings,
  3. drop empty strings,
  4. and replace single-operand concat with the operand itself.

AI-assisted-by: Augment (Claude Sonnet 4.5)

Add canonicalization patterns for `firrtl.string.concat` and
`om.string.concat` that:

  1. flatten nested `string.concat` operations,
  2. merge consecutive constant strings,
  3. drop empty strings,
  4. and replace single-operand concat with the operand itself.

AI-assisted-by: Augment (Claude Sonnet 4.5)
Signed-off-by: Schuyler Eldridge <[email protected]>

/// Remove empty string operands from concat.
/// string.concat(a, "", b) -> string.concat(a, b)
class RemoveEmptyStrings : public mlir::RewritePattern {
Copy link
Member

@uenoku uenoku Mar 21, 2026

Choose a reason for hiding this comment

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

Please use OpRewritePattern, or consider unifying these canonicalizations between OM and FIRRTL.

Copy link
Member Author

Choose a reason for hiding this comment

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

Switched to OpRewritePattern in: 9c105df

@uenoku
Copy link
Member

uenoku commented Mar 21, 2026

I think 3 can be performed at the same time as 2. 4 should be implemented as a folder.

@fabianschuiki
Copy link
Contributor

Results of circt-tests run for a7f84c9 compared to results for 1a4ee22:

sv-tests

Delta Error
0 total change

Copy link
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

Looks good

@seldridge seldridge merged commit 9fec564 into main Mar 23, 2026
8 checks passed
@seldridge seldridge deleted the dev/seldridge/string-concat-canonicalize branch March 23, 2026 22:32
@fabianschuiki
Copy link
Contributor

Results of circt-tests run for d23558a compared to results for a7f84c9:

sv-tests

Delta Error
0 total change

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.

3 participants