Closed mikeurbach closed 3 years ago
Sure, there are a couple of ways to do this: 1) You could do this with verbatim nodes, and use symbols to refer to get the renamified names. Verbatim nodes can be put at the top level of the mlir::ModuleOp and have a file name tagged on them. ExportVerilog will do the substitutions and emit the file for you.
2) You can do an ad-hoc pass like lib/Dialect/SV/Transforms/HWExportModuleHierarchy.cpp
which runs after export verilog, scrubs the the IR and does whatever you need.
The problem with approach 2 currently is that the renamified names for things are lost. My plan for how to handle this is to keep the algorithm in ExportVerilog the same, but have it slap down a "verilogName" attribute on wires, regs, etc that got renamed; It already does this for modules (which is why HWExportModuleHierarchy works).
-Chris
btw, I'd love help improving https://circt.llvm.org/docs/VerilogGeneration/ to capture some of this if it is missing :-)
Thanks @lattner, I hadn't thought about approach 1. I can look into that for this use-case. However, my impression is verbatim nodes are sort of like inline assembly, and I'm not sure if we want to build this TCL export on top of them.
What we currently have is more like approach 2. The ad-hoc pass is ExportQuartusTcl. This used to be something we could run after LegalizeNames, clean up any unsupported ops, and then run ExportVerilog, but this is no longer possible now that LegalizeNames happens within ExportVerilog.
The problem we have isn't just detecting what entities got renamified (I think if we had the "verilogName" attribute on regs, we would be all set in that regard). We are discussing a new feature for ExportQuartusTcl, and modelling the required information in the IR as a new op:
builtin.module {
msft.physical_region @region1(...)
...
hw.module {
sv.reg { region = #msft.region<@region1> }
}
}
When ExportVerilog encounters this hypothetical new msft.physical_region
, it currently emits errors. I guess this doesn't actually halt the emitter from continuing to process the supported IR, but I feel somewhat uneasy leaving unsupported ops in the IR for later passes to consume.
So to summarize, with option 1, we can build the TCL export on top of verbatim nodes, but this feels like somewhat of an escape hatch, which makes me uneasy building a core feature on top of. With option 2, we can model the new feature in a first-class way for our use case, but ExportVerilog is going to complain about the new ops in the IR, which also makes me feel somewhat uneasy.
A third option we've discussed is to "inline" the relevant information from this msft.physical_region
into attributes on the relevant instances and regs before ExportVerilog. In this way, ExportVerilog doesn't complain about any unsupported ops, and we can take the second approach of having a standalone pass after ExportVerilog. Whether we do this "inlining" of attributes or not, with option 2 we are still relying on ExportVerilog to preserve unknown attributes, which is also making me uneasy :)
In light of that, perhaps option 1 is the best way to make this TCL export work with ExportVerilog's current model. Does that sound like a fair assessment? It will require some significant reworking of what we currently have, but it might make sense if that's the most stable direction for the long term. I'm also curious if there is a better way to support other exports (like TCL) in a more first class way, so perhaps starting with verbatim nodes will give us a foothold to further generalize and improve.
I strongly dislike option 1.
Ideally, we'd be able to run all of the side-effectful "prep for verilog output" steps (primarily legalize names) without the actual exporting and these steps would ignore anything they don't recognize. We could then fork the IR and clean one up for tcl output and one for verilog output. I think an option to the export verilog pass to disable the actual text export would be sufficient (since I think export verilog only complains about unknown ops during the text output, but that may no longer be true).
If it makes it more palatable, for this specific use-case we only need ops which are unknown to export verilog at the builtin.module
level (as Mike's example implies). There is a mechanism in the pass manager to only run a pass on certain top level ops, though I don't know if that works with export verilog.
To expand on my objection to option 1:
The problem with approach 2 currently is that the renamified names for things are lost. My plan for how to handle this is to keep the algorithm in ExportVerilog the same, but have it slap down a "verilogName" attribute on wires, regs, etc that got renamed; It already does this for modules (which is why HWExportModuleHierarchy works)
Currently, we only need instances/modules so this would work. Soon we'll need wires/regs/etc. as well and your "verilogName" plan would work.
I strongly dislike option 1.
k, I think you should talk to some of the SiFive folks who are exporting thousands of text files like this with various design information using primarily option #1.
Let's talk about this at the ODM.
I would also recommend checking out this patch in progress: https://github.com/llvm/circt/pull/2066
@seldridge and @youngar do you have any thoughts here?
We use the verbatim nodes to create a whole bunch of extra collateral that we need the compiler to produce. You can see some of it in the CreateSiFiveMetadataPass
here. I would describe this approach as working very well so far.
We can't delay the generation of the metadata until ExportVerilog
because we need access to information that will be lost during LowerToHW
, namely the annotations. For this reason, without making larger changes, option 2 does not work for us.
Option 1 is very enticing because you can handle all the MSFT dialect concerns upfront without having to spread the logic to other passes. For example, you wouldn't have to encode information as new attributes or teach other passes about new operations.
We are lucky in that we usually only care about the Verilog name of things, so verbatim text replacement of symbol names works perfectly for us. Maybe it would be possibly to extend the verbatim format specifier syntax for whatever name-mangling needs to happen for TCL export?
Thanks for the feedback about option 1, that makes me less uneasy. I'll take a look at how that works now and how hard it will be to export what we need that way.
One thing Mike and I neglected to specify is that we are outputting per-instance placement data. So the tcl has to have the full instance paths given a particular "top" module. The way we currently achieve this is by doing a full instance hierarchy traversal, tracking the instance path, and outputting only the placement annotations which match the instance path. We do this after calling ExportVerilog since we need the RTL names. The difference here is that we're introducing a non-hw/comb/sv op which we would like to persist without complaint through legalize names. This is what I was ham-handedly/vacation-brained referring to by "expanded in odd ways".
Based on my outdated understanding of ExportVerilog, I don't think that's possible with just a simple format specifier.
Let's talk about this at the ODM.
Yep. I'll put it on the schedule.
Let's talk about this at the ODM.
Yep. I'll put it on the schedule.
Looks like someone beat me to it!
I strongly dislike option 1.
k, I think you should talk to some of the SiFive folks who are exporting thousands of text files like this with various design information using primarily option #1.
Mike re-wrote it this way and it appears to be working. I think some of my knee-jerk reaction was verbiage: VerbatimOp
in the sv
dialect plus the op summary/description make it seem like it's not supposed to be used how you're using it. Plus, output_file
isn't an intrinsic attribute.
Might I propose a new op named differently (CollateralOp
or something like that) with output_file
as an intrinsic attribute? Also, might I further suggest it be in the hw
dialect? Ops in the sv
dialect should only be SystemVerilog constructs, IMO. The only relationship this has to SV is that we're querying the VerilogExport renamer for names.
I have a question about how to handle unknown operations in ExportVerilog. The use-case is we may have some operations in the IR that are related to TCL export, and the TCL export has to happen after all name changes occur. Since LegalizeNames has been rolled into ExportVerilog, we need to know the name-changing side effects from ExportVerilog before we can export TCL. This is why I'm trying to run ExportVerilog while the IR still has some unsupported operations.
What is the proper path forward for this use-case? I don't see a way to run the TCL export before ExportVerilog, as long as ExportVerilog has the potential to change names. Maybe we could add an option to just do the re-naming, or an option to not emit errors for unsupported operations?
Do you have any thoughts or guidance on this @lattner ?