Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL] Add FormatStringType #8342

Merged
merged 5 commits into from
Mar 25, 2025
Merged

Conversation

seldridge
Copy link
Member

Add a format string type. This is intended to be used to improve the way we capture format strings and extend this to capturing more advanced concepts like "time" via attributes as opposed to resorting to enums and attributes.

@seldridge seldridge force-pushed the dev/seldridge/firrtl-add-printf-types branch 2 times, most recently from e68c032 to 86de666 Compare March 23, 2025 19:27
@seldridge seldridge marked this pull request as ready for review March 23, 2025 19:31
@seldridge seldridge requested a review from darthscsi as a code owner March 23, 2025 19:31
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

This is really really cool. I love the direction of this. Rust has had some good results with this approach of statically parsing format strings into a fixed data structure.

I'm wondering if we shouldn't go all in on this, and remove string interpolation from printf entirely and use dedicated formatting ops. That would allow us to define string interpolation once in a bunch of ops, and then reuse it for printf, fprintf, assert, assume, cover, and other stuff I forgot 😁

Add a new type, `!firrtl.fstring.time`, and a single expression that can
create it: TimeOp / `firrtl.fstring.time`.  These are used to model the
notion of simulation time that should be emitted into a format string.

Allow for this to be a legal substitution operand to the existing
PrintfOp.  Add printing/parsing support for this which will convert the
special substitution `{{SimulationTime}}` into a `TimeOp` and an added
substitution at the correct location.  Add emission support which inverts
this representational conversion.

Signed-off-by: Schuyler Eldridge <[email protected]>

[FIRRTL] Parse/emit special printf substitutions

Add parser and emitter support for new "special substitutions".  These are
substitutions that take the form of `{{Name}}` where "Name" is a list of
keywords defined by the FIRRTL spec.  This is intended to be used to
represent "simulation time" and "hierarchical path" and will be compiled
to `$time` and `%m` in Verilog.

Signed-off-by: Schuyler Eldridge <[email protected]>
@seldridge seldridge force-pushed the dev/seldridge/firrtl-add-printf-types branch from e9cd52c to db20bd8 Compare March 25, 2025 15:27
@seldridge seldridge merged commit 0802c2d into main Mar 25, 2025
5 checks passed
@seldridge seldridge deleted the dev/seldridge/firrtl-add-printf-types branch March 25, 2025 18:33
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