[Verif] Add format_verilog_string, print operations
#5616
Conversation
#5577, #5575 Re. the lowering; `verif.printf` lowers directly to a `sv.fwrite` via. inspection of its source operand (which is expected to be a `verif.fstr`). The source `fstr` is `not` deleted in the process (there may be other users). However, i marked `verif.fstr` as `Pure` meaning that it'll get removed if `canonicalize` is run after `lower-verif-to-sv` and there are no more users of the result. It's expected that `verif.printf` will reside inside *something* that eventually lowers to a procedural SV region, if one is going the SV route. I'd imagine that most users will embed `verif.printf` inside `hw.triggered` regions.
| def PrintfOp : VerifOp<"printf", []> { | ||
| let summary = "Prints a formatted message."; | ||
| let arguments = (ins HWStringType:$formattedString); | ||
| let assemblyFormat = [{ | ||
| $formattedString attr-dict | ||
| }]; | ||
| } |
There was a problem hiding this comment.
I think I would just call this PrintOp and verif.print, without the "f". This operation doesn't really do any formatting, it just prints a string. The formatting happens with the above op. It would be cool if the formatting didn't leak into the printing op!
We could argue that this being emitted as sv.fwrite is just an optimized lowering that's able to lower PrintOp(FormatStringOp(...)) directly. But we could also just lower the FormatStringOp into a fully interpolated SV string first, and then do an unformatted print afterwards.
There was a problem hiding this comment.
Agreed. reserve printf for when we have file handles or something.
| auto fstrOp = dyn_cast_or_null<FormatStringOp>( | ||
| op.getFormattedString().getDefiningOp()); | ||
| if (!fstrOp) | ||
| return op->emitOpError() | ||
| << "expected FormatStringOp as the source of the formatted string"; |
There was a problem hiding this comment.
This looks like a good starting point! In the future we might have something like hw.constant_string which we could accept here as well. Or instead, absorbing FormatStringOp could be an optimized lowering and the default simple lowering would just be the !hw.string being passed into the sv::FWriteOp as is.
There was a problem hiding this comment.
I agree -- hw.string should be converted to a sv string (in export verilog) and the default lowering for fmtstring should be $*format to make this more composable.
| auto fstrOp = dyn_cast_or_null<FormatStringOp>( | ||
| op.getFormattedString().getDefiningOp()); | ||
| if (!fstrOp) | ||
| return op->emitOpError() | ||
| << "expected FormatStringOp as the source of the formatted string"; |
There was a problem hiding this comment.
I agree -- hw.string should be converted to a sv string (in export verilog) and the default lowering for fmtstring should be $*format to make this more composable.
| let summary = "Creates a formatted string."; | ||
| let description = [{ | ||
| Creates a formatted string suitable for printing via the `verif.printf` op. | ||
| The formatting syntax is (for now) expected to be identical to verilog |
There was a problem hiding this comment.
Perhaps be explicit in the name? FormatVerilogStringOp and format_verilog? If verilog is leaking in, lets be clear about it and then when we define an independent format string, it can have it's own op rather than having to convert all the users of this one.
There was a problem hiding this comment.
Agreed. Since printf accepts a hw.string, it shouldn't be a problem to add something else later.
| def PrintfOp : VerifOp<"printf", []> { | ||
| let summary = "Prints a formatted message."; | ||
| let arguments = (ins HWStringType:$formattedString); | ||
| let assemblyFormat = [{ | ||
| $formattedString attr-dict | ||
| }]; | ||
| } |
There was a problem hiding this comment.
Agreed. reserve printf for when we have file handles or something.
| ConversionTarget target(context); | ||
| RewritePatternSet patterns(&context); | ||
|
|
||
| target.addIllegalOp<PrintfOp>(); |
There was a problem hiding this comment.
FormatStringOp can be lowered to $sformat to cover everything.
|
Thank you for the review - updated the format string op to be called I'd like to do this the "right" way - that is, emit So I'll mark that as future work and go with this for now. |
mortbopet
changed the title
fstr, printf operationsformat_verilog_string, print operations
#5577, #5575 - relatively small changes, so decided to just do both of these in one go.
Re. the lowering;
verif.printflowers directly to asv.fwriteoutputting tostdoutvia. inspection of its source operand (which is expected to be averif.fstr). The sourcefstris not deleted in the process (there may be other users). However, i markedverif.fstrasPuremeaning that it'll get removed ifcanonicalizeis run afterlower-verif-to-svand there are no more users.It's expected that
verif.printfwill reside inside something that eventually lowers to a procedural SV region, if one is going the SV route. I'd imagine that most users will embedverif.printfinsidehw.triggeredregions.