I wanted to discuss this at the meeting yesterday but we ran out of time.
We had touched on this briefly in our Oct 14th meeting. Here’s the recap IIRC: There are two different ways we could “lower” to SystemVerilog code: 1) provide an Interface for Ops and Types so they can worry about it themselves. 2) Make everything lower to the SV dialect before outputting.
My thoughts: Interfaces provide the most flexibility and can be used along with types/ops in the SV dialect for common SystemVerilog constructs (e.g. SV Interfaces). This prevents us from having to write a complete (for some definition of complete) syntax tree in the SV dialect when the slightly-higher level Ops in the RTL dialect suffice. (How much value is there in having an AndOp defined both in the RTL and SV dialects?)
Assuming everyone likes this hybrid approach, what should this interface look like? The strawman is just pass around a raw_ostream and have code write raw text. A few possible improvements and problems with that strawman:
SystemVerilog (like every language except perl) has syntactic restrictions. Do we want to enforce the syntactic restrictions? (I think this it the primary advantage of requiring everything to lower to the SV dialect.) If so, how should we do that?
Some Ops have to output code in various places around the output code. For instance, declaring wires before using them is often necessary.
I think there is value in providing a SystemVerilogPrinter which knows about basic SV syntax and provides SV specific helper functions. (Kind of like DialectAsmPrinter.) What should this helpers API look like?
There’s information which needs to be shared both globally and at particular scopes. (e.g. allocation of text identifiers.) Should we handle this in the proposed printer?
The other question is how do we structure this in terms of code structure logistics. My preference would be developing a new system which exists in parallel with EmitVerilog.cpp, stealing as much as possible from EmitVerilog.cpp. This has the advantage of not worrying about breaking something in EmitVerilog.cpp.
@clattner this kind of thing is probably present in other places in LLVM. (The MC asm printers are probably a rather simple example.) Can you provide some code pointers and/or suggestions for this? Can you speak to the advantages / pitfalls of DialectAsmPrinter or other such things?
I’d like to get the ball rolling on this since I’m about to start emitting SystemVerilog for ESI and don’t want to be writing throw-away code or for the API to change. I’m even willing to throw together a prototype and iterate on it a bit.
I was thinking about this too… Still am, really. I think I am in agreement about the hybrid approach.
On one hand, there seems to be value in having well-defined constructs in the SV dialect. To me, it is nice to be able to simply lower to the SV dialect ops/types using the pass infrastructure and not think about printing. I would expect to go this route for things that are specific to System Verilog, but not HDLs in general.
I also see value in having an interface-based approach for ops/types to print as System Verilog. As you mentioned, the RTL dialect could use this interface to define how to print all the stuff that is generic to structural HDL and not specific to System Verilog. One day, if we had a similar interface for printing VHDL, I would expect a good chunk of that to be implemented at the RTL dialect as well.
Having an interface also provides a well-defined escape hatch for other dialects that may just want direct control over how their ops/types emit as System Verilog. I think it’s possible this could be really useful at times… I can also see how it could be abused to skip all sorts of layers of abstraction and verification.
I’m still thinking about exactly what such an interface would look like, but I have written a couple Verilog printers so I will try to condense my thoughts into something useful and share them.
Thanks for raising this - I think it could be good to talk about it next week (and we should probably have these discussion topics before planned talks :).
I’m not sure what you mean by interfaces here: do you mean MLIR operation interfaces, or SV Interfaces? There is a name collision going on :-). I agree with you that we eventually want to generalize the EmitVerilog code to be parameterized with OpInterfaces.
I expect the general design to look similar to the custom op printer for MLIR - the operation-specific hook will be passed a c++ class with a bunch of helpers and other stuff that provides structure to the problem. Similar to DialectAsmPrinter as you mention.
However, the way you’re explaining this above makes it sound like a good SV dialect is in conflict with OpInterfaces in verilog emission. I don’t see it this way, for the same reasons that having a “standard dialect” is useful even though you want to support custom things.
I am an incrementalist on this though. I would really like to fix up the SV/RTL dialects and get the EmitVerilog code to not know anything about FIRRTL. This will give us a baseline model that we can parameterize and expose through OpInterfaces. I don’t think that ignoring the existing code is going to be the right way to go, because the design space is vast.
For context, the incremental approach is how the asmprinter/parser logic evolved over time. Originally things like Affine dialect and functions were built in, they were progressively refactored out.
The way I’m looking at it, the SV dialect and the new OpInterface aren’t in conflict at all. Couldn’t the SV dialect eventually use the new interface for System Verilog emission? Seems like a way to dogfood the new interface while refactoring EmitVerilog.
I agree that would be a good outcome in theory, I’m just concerned that it will drive premature generalization. Why wouldn’t we want the SV dialect to be able to describe most/all things? Once that exists, why do you want the additional abstraction here?
Let me be more explicit about this, just using the previous conversations about the RTL dialect as an example:
The RTL dialect is designed to be easy to analyze and transform, not to map directly onto verilog. That means that an EmitVerilog pass over it will need to have complexity to handle the implicit extensions and truncations that verilog does (inserting explicit ones, and ignoring ones that verilog does).
An alternative design is to lower from rtl.and to an sv.and that directly models verilog semantics. Doing so doesn’t have any additional expressive qualities in theory, but has a really powerful benefit: you can then use dataflow analysis and compiler passes to transform the lowered form. This is the power of the SV dialect, and I don’t think that an OpInterface has the same qualities.
On the other hand, the OpInterfaces are more extensible and hackable, so it is good for enabling research directions etc. These seem like complementary benefits, not exclusionary design points.
I mean MLIR OpInterfaces. There are several words and acronyms which are heavily overloaded in software alone, than you add the hardware jargon… The one that gets me is PR – pull request or partial reconfig?
I didn’t mean to imply that. I do, however, want to avoid redundancy wherever possible (at least in the short term. I think (not strongly) that the RTL dialect should do the heavy lifting and (at least in the short term) the SV dialect should be related to SV-specific vagaries – like SV interfaces. I don’t see a need to be able to express assign a = b | c in two different ways in the short term. (I could see the advantage for customizing the printing and such down the road.) I don’t like duplication of work.
In general, I agree. There are, however, two logistical issues with that:
The FIRRTL logic and RTL/SV logic are heavily intertwined in EmitVerilog today and I don’t know enough about FIRRTL to be to (quickly) know what’s necessary for FIRRTL. So I’m worried any potential changes will break FIRRTL.
The ESI dialect will need to ability to output verilog and there is some custom code it needs to be able to output. (For instance, I’m gonna need a struct type which I’ll want to play around with before it moves to the RTL dialect.) I don’t want (as per your suggestion) to let ESI-specific code into the EmitVerilog logic.
Also, I don’t know how we cleanly separate the FIRRTL verilog emission from the SV/RTL stuff (while still sharing code) without an OpInterface.
I’m not so much arguing for ignoring the existing code and starting fresh as copying the SV/RTL dialect stuff and refactoring it while not touching the existing EmitVerlog code for reason 1 above.
Let me propose something a little bit more incremental. Each of these steps would be a PR:
Fork EmitVerilog.cpp and go to town ripping out the FIRRTL stuff while keeping the RTL/SV stuff with refactoring wherever it makes sense. This would enable someone more familiar with FIRRTL to clean it up separately without complicating or blocking the first step.
Start a Verilog printer class and MLIR interface and grow it incrementally. I need the ability to output types other than wires, so that’s a good starting point IMO.
Grow it as necessary to support development as needed.
Refactor whatever is left of the old RTL/SV verilog emission code to use the new stuff.
Of course, this would all be easier if we had a SytemVerilog compilation and test flow to ensure that our emitted SystemVerilog is correct. I’m planning on adding Verilator and setting up a test flow but that’s a different topic.
I agree; however, would the rtl.and and sv.and really have different semantics? I assume not so why have two? Like we all seem to violently agree, the SV dialect and MLIR interfaces are complementary so this is kind of a minor nit. (And we may be bikeshedding on this particular detail without much experience/info or shared context.)
I read both of these as “I don’t want to wait for the refactoring work to get done”. This is reasonable, I think the answer is that you should have an ESI-specific equivalent to EmitVerilog so you can do pursue the research direction without being blocked on this, it doesn’t mean we should prematurely generalize and complicate the EmitVerilog stuff. It already has too much going on
The goal is for FIRRTL dialect to lower to SV/RTL completely, so there is no support for FIRRTL in EmitVerilog. This is the “LowerToRTL” and “LowerToRTLModule” work that is going on. Once that completes (hopefully over the next few weeks?) the FIRRTL support will be just deleted from it.
I’m ok with this so long as it lives in the ESI dialect directory. This gives you the ability to prototype the verilog printer interfaces etc without worrying about breaking other stuff, when you get to a good place, we can discuss the concrete design.
Yes. Verilog syntax has all sorts of weird behavior, like implicit extensions and truncations that we don’t want in the RTL dialect. sv.and is perhaps too simple of an example to convey this.
A better example is synthesization of sv.casex nodes. This is something that should exist in one place - we shouldn’t have every dialect try to print out casex nodes themselves, they should lower to the RTL dialect, then have a “prepare for verilog emission” pass that goes and “raises and cleans up” the IR to enable pretty emission.
Yeah, that’s about right. I’ve got deliverables that I’m waaayyyy behind on and would rather not fork and develop a lot there.
That makes sense. I didn’t realize this would happen so soon.
This seems like a good compromise, especially since I’ll be prototyping types and ops in ESI which will be candidates for the RTL dialect down the road.
