LLVM Discussion Forums

FIRRTL aggregate types discussion

During Adam’s talk today, it seemed like there was a good deal of interest in the topic of aggregate types, but we ran out of time. I’m personally very interested, so maybe we can continue the discussion here?

At the moment, the FIRRTL dialect in CIRCT has a BundleType, which the lowering from Handshake to FIRRTL leverages to bundle a data signal with its valid and ready bits. However, the BundleType isn’t fully supported in the EmitVerilog module. EmitVerilog has some places that flatten down the bundle, but, notably, there is no handling of BundleType in a module’s port. This means the path through Handshake to FIRRTL won’t be able to actually emit Verilog. (This is my understanding of the current state of CIRCT, please correct me if there’s a better path from Handshake to emitting Verilog).

In the discussion today, someone (@jdd?) brought up the connection to SystemVerilog interfaces. @stephenneuendorffer has previously mentioned that handling aggregate types might be heading in the direction of SystemVerilog interfaces, and we have a proposal to add interfaces to the SystemVerilog dialect here. Once that lands, one possible approach might be to lower from BundleType at the FIRRTL level to interfaces at the SV level. Another approach might be to flatten the BundleType earlier on as a lowering step, but that seems to lose some structure that might be useful to preserve until we are ready to emit Verilog.

I think there was already some discussion about this before I tuned in to this project, but after the talk today, it seems appropriate to continue the discussion. I’d love to hear about some lessons learned from FIRRTL, as well as any other experiences with handling aggregate types. My hope is after some discussion, we can come up with a more concrete path forward for CIRCT.

In my opinion, anything that is bidirectional should get boiled down to a SystemVerilog interface (since they have a notion of directionality), which it sounds like bundles are. For other aggregate types (that have no notion of directionality), I think SystemVerilog types are appropriate.

This is my understanding as well.

Here’s an example BundleType generated by lowering from Handshake to FIRRTL:

!firrtl.bundle<valid: uint<1>, ready: flip<uint<1>>, data: uint<64>>

I didn’t see any documentation about this in CIRCT, but the FIRRTL spec discusses this in Section 4.3:

In a connection between circuit components with bundle types, the data carried by the flipped fields flow in the opposite direction as the data carried by the non-flipped fields.

I think there are two issues here:

  1. The FIRRTL dialect is really a “low FIRRTL” dialect. Low FIRRTL is approximately SSA, has aggregates flattened, and removes the more complicated IR structures like conditional statements (when/else). It would be good to support higher levels of FIRRTL inside the FIRRTL dialect and then add support for lowering to low FIRRTL. In effect, you should be able to use either the extant Scala implementation of the FIRRTL compiler or the MLIR-based one.

  2. The fact that FIRRTL flattens aggregates as part of its lowering process is problematic for emitting SystemVerilog structs or interfaces. I’ve thought a bit about how to do this for the Scala implementation of the FIRRTL compiler (storing aggregate structure in an annotation and then reconstructing the bundle before Verilog emission). However, it probably makes more sense to not lower to low FIRRTL in MLIR and instead do a conversion to the SystemVerilog dialect.

I think both of these are complementary approaches that should likely happen at some point.

This approach makes total sense to me. I think CIRCT has some high-to-mid FIRRTL constructs already, e.g. the when operation and bundle type. However, I didn’t see how to lower them to low FIRRTL (e.g. flatten bundles) as a pass within the dialect (please correct me if I’m wrong). There are some places that bundles are flattened in EmitVerilog, but I think we are talking about doing a lowering within the FIRRTL dialect before that. This is definitely a clear path forward.

My two cents is if there are higher level FIRRTL constructs that have a good analogue in SystemVerilog, it is worth lowering directly from the FIRRTL dialect to the SystemVerilog dialect. I think we can agree this approach is attractive, since we can maintain the higher level structure in the IR until the very end.

Given the flexibility of MLIR, we can definitely implement both of these approaches, but it seems the second approach is preferable. I’d like to explore this route and come up with a more concrete plan for what that might entail.

Sorry I missed this thread (my fault, just busy and got buried in my email).

A few things - the FIRRTL dialect is intended to model the FIRRTL type system and operation set pretty directly, it isn’t intended to be necessarily the “best” (whatever that means) way to model hardware. The design here is fairly constrained by the existing FIRRTL ecosystem, which is just one of the things that should talk to CIRCT.

One correction above:

The FIRRTL dialect actually supports (or at least, is intended to support) all levels of FIRRTL. It already has parser support for when clauses, aggregate types, unknown widths, etc. There is no support for the various inferences and lowering passes that are part of FIRRTL, but those should be built out as we have engineering time to work on it.

Yes, +1. I think we should design the “RTL” type system independently of FIRRTL and then lower from FIRRTL to it. The RTL type system can be more general than what FIRRTL generates today, including support for interfaces or other type system things if that makes sense.

Yeah makes sense. I’d like to see the Chisel stack generate more readable verilog, including preserving more high level types. This can be a parallel or follow-on track to getting the abstractions right within FIRRTL/RTL/SV dialects though.

-Chris

Yeah, I’m wrong here. The FIRRTL dialect seems to parse and represent everything, including “CHIRRTL”-specific things (smem and cmem)—Chisel extensions to the normal FIRRTL IR that have historically made the Chisel to FIRRTL emission easier, but have long been marked for eventual removal.

As @clattner mentions, it’s the lowering that’s missing. Getting this up and running is some work, but will happen. In the meantime, the extant Scala implementation of the FIRRTL compiler can be used as a standalone binary for doing lowering for pieces that are not yet available.

Yep, it is even bug-for-bug compatible with some weird things :-). I think that FIRRTL added support for some verification constructs recently though - those are not supported by the parser and IR yet. I’ll file a bug to track that.

I have been taking a look at this hypothetical example from the SV interfaces PR as a lowering target for aggregate types in FIRRTL. So far I’ve pulled together enough to transform some IR like this:

firrtl.circuit "dataflow_example" {
  firrtl.module @dataflow_example(%in: !firrtl.bundle<valid: uint<1>, ready: flip<uint<1>>, data: uint<64>>,
                                  %out: !firrtl.bundle<valid: flip<uint<1>>, ready: uint<1>, data: flip<uint<64>>>) {
  }
}

Into this:

firrtl.circuit "dataflow_example" {
  sv.interface @i2086446170726900693  {
    sv.interface.signal @valid : ui1
    sv.interface.signal @ready : ui1
    sv.interface.signal @data : ui64
    sv.interface.modport @mp16117715832878755040  ("input" @valid, "output" @ready, "input" @data)
    sv.interface.modport @mp12676913513676193433  ("output" @valid, "input" @ready, "output" @data)
  }
  firrtl.module @dataflow_example(%in: !sv.interface_modport<i2086446170726900693, mp16117715832878755040>,
                                  %out: !sv.interface_modport<i2086446170726900693, mp12676913513676193433>) {
  }
}

I can clean up my branch and push the proof of concept up to GitHub, but I’m curious if anyone has feedback on the utility of having such a pass. It is intended to lower bundles generated by HandshakeToFIRRTL towards something that EmitVeilog can process.

The new sv.interface_modport type gets inserted for every port of bundle type, and then it is up to the rest of the FIRRTL module to determine the semantics. I’ve been thinking about this hypothetically, but this is the first pass where I’ve put to use the new SV operations and type.

I’m curious if anyone has feedback on the above, and I’m also thinking about what directions this IR can go from here.

First and foremost, I’m trying to lower a FIRRTL circuit into something I can emit as System Verilog. Within EmitVerilog, it looks like we will need to add support for the new SV operations and type, and then thread through correct handling in some FIRRTL operations (instance, subfield, connect, maybe more). This is what I’m exploring now–if we can get from the above IR to correct, readable System Verilog.

I’m also looking into the new LLHD pass to lower from FIRRTL, and I’m seeing a need for “something” that can represent the aggregate types. I’m not as familiar with the LLHD dialect, so I’m not sure if the additions to the SV dialect and this lowering are useful to LLHD or not. It may make sense for LLHD to have it’s own representation here, but perhaps it can share something with the SV dialect.

I think similar questions come up in the lowering from FIRRTL to the RTL dialect as well. Do aggregate types have a place in the RTL dialect, or should they be lowered first? Should the RTL dialect depend on the SV dialect type system as a way to support aggregate types?

One overarching question I’m wondering at this point is whether people see utility in having the new operations and type in the SV dialect? It seems like that is the designated place for things that closely model System Verilog at the AST level, but we haven’t actually put any types into an SV type system. Does it make sense to start here?

I think it would be great to have some common tooling/infrastructure to facilitate flattening out such nested types. For things like a simulator or a synthesizer, I’m pretty sure that aggregating signals into structs/bundles/interfaces is more of a “debug information” kind of thing, rather than something the tool can directly leverage. My guess is that the dialects at a low level of abstraction (cables and drivers/gates rather than interfaces and producers/consumers) would want to have somewhat flat and simple types. But at a higher level it’d be great to have such bi-directional aggregates around – to help emit better SV and maybe catch directionality errors?

Aggregates in the RTL Dialect

Regarding whether the RTL dialect should depend on the SV dialect types, I’d argue that the SV type system is actually not very good at anything beyond simple multi-dimensional wires. Interfaces have gained some tool support recently, and they do offer a form of bidirectionality and signal bundling, but nesting interfaces is not well-supported. Also, the concept of modports is geared towards testing and verification (e.g. regular input/output modports, plus an all-input “bus checker” modport), and in the context of an SSA RTL dialect with proper def-use it’s not even quite clear how you would wire up an interface with three modports (who’s the third party?). In practice I would expect interfaces to always have two exactly complementary modports; seems like overkill if all we want is to have aggregation alongside the notion of a producer-consumer relationship.

In my opinion FIRRTL’s concept of bundles and bidirectionality would be a better fit for the RTL dialect. The concept of “here’s an aggregation of wires” and “expect these to flow in the opposite sense of traffic” seems to me to be more succinct and have less “surface area”.

FIRRTL Bundles in LLHD

As @mikeurbach mentioned, I’ve started to look into a lowering from FIRRTL to LLHD:

LLHD currently does not support bidirectional structs/bundles, in order to keep simulation and hand-off to a synthesizer easy. SystemVerilog interfaces pose a similar issue as FIRRTL bundles, due to their bidirectional nature. Although FIRRTL seems to take this a step further by allowing nested bundles/flips – SV has some support for nested interfaces in the standard, but I doublt that is well-supported in commercial tools.

The Moore compiler currently flattens out SystemVerilog interfaces when emitting LLHD, which can be a bit nasty if there are arrays of interfaces. I figure with nested interfaces this would become even more involved. Currently the following SystemVerilog (copy-paste into www.llhd.io):

module foo (
    bar.in x,
    bar.out y[3:0]
);
endmodule

interface bar;
    logic [31:0] data;
    logic valid;
    logic ready;
    modport in (input data, valid, output ready);
    modport out (output data, valid, input ready);
endinterface

Translates to the following LLHD:

entity @foo (
    i32$ %x.data,
    i1$ %x.valid,
    [4 x i1]$ %y.ready
) -> (
    i1$ %x.ready,
    [4 x i32]$ %y.data,
    [4 x i1]$ %y.valid
) {
    ...
}

LLHD has an added level of nasty by separating inputs from outputs – although I intend to get rid of that in favor of some "input"/"output" annotation as you do in the SV dialect.

Considering a nested FIRRTL bundle, like something along the following lines:

firrtl.circuit "foo" {
  firrtl.module @foo(%x: !firrtl.flip<bundle<
      payload: bundle<
        req: uint<32>,
        rsp: flip<uint<4>>>,
      valid: uint<1>,
      ready: flip<uint<1>>) {
    ...
  }
}

We probably want some FIRRTL-to-FIRRTL pass which gets rid of the bundles, and resolves the subfield ops accordingly, as you guys already discussed on Discourse:

firrtl.circuit "foo" {
  firrtl.module @foo(
    %x.payload.req: !firrtl.flip<uint<32>>,
    %x.payload.rsp: uint<4>,
    %x.payload.valid: !firrtl.flip<uint<1>>,
    %x.payload.ready: !firrtl.uint<1>
  ) {
    ...
  }
}

It would be great to have such a “single-flip-no-bundles” representation of FIRRTL. This should then be a pretty straightforward conversion to LLHD:

entity @foo (
    i4$ %x.payload.rsp,
    i1$ %x.payload.ready
) -> (
    i32$ %x.payload.req,
    i1$ %x.payload.valid
) {
    ...
}

When converting to an SV dialect, one could simply not do that transformation, to leverage the higher-level correspondence of bundles to interfaces.

I guess this is the use-case that I’m pushing on right now. Is it worth adding these ops and type into the SV dialect so there is one standard way to model SV interfaces in CIRCT, if your compiler needs it? I’m thinking the answer is yes, based on previous discussions. Would LLHD or RTL support such ops and type? I’m starting to think the answer is no, and that’s ok.

This is a fair point. I’ve been experimenting with FIRRTL modules generated from the Handshake dialect, and my pass will generate a single interface with 2 modports for each unique datapath width + the handshake signals. So in a simple circuit with only, for example, uint<64> datapaths, there are really only two interfaces: a handshake with no data, and a handshake with a uint<64> data signal.

Thanks for the feedback. The more I think about it, the more it seems like the RTL dialect may want its own representation, if it is going to deal with aggregate types at all.

This makes sense to me. If we work on a FIRRTL-to-FIRRTL lowering that simplifies everything into a “low-FIRRTL” dialect, that seems to make the conversion to LLHD more straightforward.

I have been looking at the conversion from FIRRTL to RTL as well, and if we did such a lowering within the FIRRTL dialect first, it would also simplify the lowering to RTL (which currently barfs when you try to connect bundles).

Yep this is how I have been thinking about it as well.

So at this point I’m considering the utility of 3 things: the ops and type for SV interfaces, a pass to lower bundles into SV interfaces, and a pass to lower bundles into simpler types. The first two are what I’ve been working on here, with an eye on generating a SV module that uses interfaces in its modules’ ports. The third item seems like the best way to transform bundles into something the LLHD and RTL dialects could consume today, which then has a clear path to SV.

Agree. Getting all mandatory FIRRTL IR lowering into CIRCT would open up some more concrete activities. This is potentially a big task, but the steps are known (as it’s a reimplementation of the Scala FIRRTL compiler).

The specific “single-flip-no-bundles” representation is a piece of low FIRRTL that should be enforced after the Scala FIRRTL compiler’s LowerTypes pass—all aggregates are flattened to ground types.

I’m of two minds on this.

One the one hand I think getting a basic FIRRTL compiler up and running inside MLIR makes sense. Any extant language targeting FIRRTL IR can then seamlessly switch compilers (they currently can’t because, as you identify, lowering in FIRRTL isn’t implemented). Having two implementations online then helps test the new one with the old one.

On the other hand, and longer term, eventually I’d hoped that some “nexus” dialect emerges through which all “circuity” dialects typically go. RTL dialect seems like the most likely candidate here. So, I’d expect things to swing towards FIRRTL as a load bearing path, but then to have RTL dialect start peeling away and replacing pieces of FIRRTL lowering. E.g., if you can go directly from middle FIRRTL (which has aggregates) to RTL dialect, then trivial aggregate emission is possible (whether it’s interfaces, structs, or whatever).

I’m in agreement with all of the above.

I haven’t looked into the LowerTypes pass in Scala, but I’m starting to think it is worth porting that to CIRCT. In the longer term, as you mention, we might go different directions. But, pragmatically speaking, we are already hitting paths within CIRCT that get hung up on how to handle FIRRTL aggregate types, and I would like to unlock these paths in a way that isn’t “hacky”. Porting the LowerTypes pass to the FIRRTL dialect seems like a clear way to unlock emitting System Verilog, lowering to LLHD, and lowering to RTL with the current state of CIRCT.

Maybe we don’t lean on this pass in the long term, but for now, it seems like a well-defined and not “hacky” way to unlock this functionality. I’d be interested in working on this if we agree it is a worthwhile addition (and no one else is already looking into it).

I do see how the RTL dialect should hopefully emerge as a “nexus” dialect that circuits go through, so perhaps it is worth discussing what it would look like to go from FIRRTL aggregates to the RTL dialect. We could lower aggregates within FIRRTL first, as discussed above. Or, we could come up with a way to represent this in RTL. Would a representation like bundles (as Fabian mentioned) make sense? Would it make sense to depend on the SV dialect in the RTL dialect and use the proposed SV interfaces? I’m curious to flesh out what the ideal solution will look like in the long term, even if we are making more pragmatic decisions in the short term.

To help clarify my thoughts here, I’ve put together this little diagram. The passes that exist today are solid arrows, while the passes we have been discussing are dotted. I hope this is an accurate picture–this is what is in my mind while I’m thinking about the passes my compiler will invoke.

Over the weekend, I posted some examples from my work to prototype 6. using the SV dialect interfaces proposal. We’ve since discussed 2., 3., 4., and 5. Hopefully this helps clarify for others… just drawing this out was helpful for me.

Diagram looks great! I am highly in favor of solidifying all the dashed lines with Mid FIRRTL IR -> Low FIRRTL IR being tackled first.

I would add one modification to the diagram (new point of confusion).

There is technically a “higher” form of FIRRTL IR before High FIRRTL IR (“CHIRRTL”) that has Chisel semantics for memories (cmem for combinational read memories and smem for sequential read memories). The read/write ports of these Chisel memories are then inferred based on use. A left hand side usage is a new write port. A right hand side usage is a new read port. Inside the Scala FIRRTL compiler these are quickly converted to High FIRRTL IR memories.

I don’t think anybody other than Chisel is emitting these constructs, but Chris added support for them (to be parsed and represented in the FIRRTL IR dialect). So, there’s probably another box “CHIRRTL” and a dashed arrow into High FIRRTL IR.

Makes sense about CHIRRTL, thanks for clarifying.

I’m ready to start helping with the High/Mid FIRRTL -> Low FIRRTL lowering. I have two concrete lowerings in mind–flattening bundle types and the when operation. With those done, I should be able to lower FIRRTL IR generated from Handshake all the way to System Verilog.

Should I open issues on GitHub to track these enhancements? I’m not sure how many people we are coordinating here, so if it’s sufficient to say I’m working on it here, then that’s fine by me.

BTW, I am still exploring the arrow marked above as 6. in parallel. If anyone other than me is interested in this approach, I can share the prototype. For now, I think I will focus on lowering within the FIRRTL dialect, since it seems more useful to the rest of the CIRCT community in the short term.

This would be a big help!

I think these two lowerings are probably sufficient for a minimal example. The full set of passes that run for each lowering in the Scala FIRRTL compiler can be found in firrtl.stage.Forms.

Removal of when statements is a property of middle FIRRTL IR currently handled by the ExpandWhens transform. This is known to be a somewhat complicated transform.

Flattening of aggregates is a property of low FIRRTL IR currently being handled by LowerTypes. This is pretty straightforward.

It was brought to my attention that RemoveAccesses (conversion of a dynamic index into a mux) is probably more important than ExpandWhens as it is more likely that users are using sub accesses in FIRRTL dialect than when statements.

All that said, it is totally fine to rethink how all this should be constructed and not be limited by the architectural decisions of specific passes in the Scala FIRRTL compiler. Namely, if there’s a better way to do something in CIRCT we should do it. E.g., RemoveAccesses could potentially be better handled with more standard compiler theory handling of arrays.

Issues would be good!

I’ve been looking at some whens that come from the Handshake conversion. Perhaps we can generate a different construct there. If we don’t really need a FIRRTL when there, maybe we don’t need to worry about implementing ExpandWhens or RemoveAccesses in CIRCT for now.

I’ve filed an issue about lowering the bundle types here.

Catching up from the weekend…

We actually use SV interfaces and SV types quite extensively. On the interfaces side, yes it’s mainly (if not entirely) two complimentary modports. In terms of SV types, structs and unions are actually quite useful to SV devs. They allow you to do single assignments and slice 'n dice data symbolically. Unions are actually (IMO) superior to C unions in that they are discriminated. So it’s got a lot of the basics and they’re actually well supported by recent-ish tools.

Also, we need to consider integration with existing RTL. IME, this is critical for adoption. The more type information which can be preserved through lowerings, the easier integration with existing RTL will be. (This is exactly the problem which ESI intends to solve.)

As such, why put types in the SV dialect at all? As long as there’s sufficiently high-level data types (presumably in the RTL dialect) which can be shared (e.g. struct, union, primitives), why not just lower (if necessary) to the ones that emitVerilog supports?

Out of curiosity (and purely out of curiosity), why go Handshake to FIRRTL instead of Handshake to RTL directly? Is it because there are high-level constructs in high/mid FIRRTL which make it easier to lower? (I’m try to figure out which dialect I should target in the future.)

~John