-
Notifications
You must be signed in to change notification settings - Fork 299
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
[RFC][Sim] Add format string type and format specifier ops #7208
Conversation
db990ae
to
36e76dc
Compare
I love the idea of actually breaking the formatting string up into separate ops that then get concatenated. Especially since it makes the format string syntax itself a frontend language concern, which gets lowered to a bunch of ops, which can then get emitted appropriately by the backend. The FIRRTL printing ops basically accept "whatever SV format string you want". With this, we could actually specify formatting strings in FIRRTL that are parsed and lowered. This feels a lot like what Rust did with its |
Thanks a lot, @fabianschuiki. I'm still hesitant to push (partially) redundant infrastructure without being able to sketch out a clear path to unification. Maybe @mortbopet or @teqdruid could briefly chime in on their use of the current |
@prithayan is sim.printf used internally? |
I don't think we use the verif dialect. |
@darthscsi , We don't use the |
Thanks for the feedback. Since there are apparently no known users of the To be honest, using format strings for parsing (like To make some progress here I would merge this PR in the coming hours, unless anyone has any objections. Fell free to leave a post merge review or voice your concerns in the inevitable follow-up PRs. |
36e76dc
to
17c9285
Compare
Really cool! 🥳 Interesting thought about scanf-like parsing of strings. This feels like you'd want a complementary set of parsing operations, and then either the frontend emits those directly, or maybe there's a pass that transposes the string concats into sequences of parses? Or treats them like a regex? Not entirely sure. The parsing/scanf is also harder because we don't have the liberty to assemble the string separately and pass it into a Verilog construct, like we have for printing. Although, the plusargs could theoretically accept any string and the parsing could then be done separately… |
The motivation of this PR is to get a frontend/backend agnostic representation of "printf-like" format strings into the core dialects. These should allow us to specify the emission of messages during simulation, which include formatted representations of runtime values.
It adds the
!sim.fstring
type to the IR, which represents a sequence of format tokens. These tokens can either be string literals or the combination of a (SSA) value and a format specifier. It also adds the usual suspects of format specifiers as individual operations: Binary, signed/unsigned decimal and hexadecimal. To make the transition of the current direct FIRRTL-to-SV lowering as simple (and compatible) as possible, the specifics of those heavily lean on the way formatting works in SystemVerilog. Currently formatting is limited to integer values. For the details I'd ask you to refer to the documentation in the TableGen source.I deliberately decided against a representation where substitutions are embedded within a string, e.g.,
"Foo = %x"
. While this imposes some efforts on the frontend lowerings to tokenize their format strings into separate operations , I think this will help us to avoid confusion between different fronend and backend formats. And more importantly, it makes processing substitutions in an "MLIRish way" easier.These additions are intended as a first step towards a FIRRTL->Sim->Arc/LLVM-IR lowering pipeline for
printf
statements.I've got a functional prototype of this integration over in my repository, which can hopefully give you an impression on how this could look like in the end.
I have to point out that this is effectively a competing implementation to the existing print operation in the Verif dialect, added in #5616. I hope this does not disqualify me from the title of friendly neighbourhood compiler engineer. I'm currently not aware of any (publicly available) lowering to
FormatVerilogStringOp
, so I cannot judge how hard it would be to transition this to my implementation. But I am somewhat confident that we'll find a way to unify these.I'd be happy to hear your opinions and suggestions on this approach.