[major] Add reference types.

[dtzSiFive]

Summary

Introduce "reference types" for indirect access to FIRRTL constructs.

Add operations for generating probe references and using them to remotely access data (via XMR's).

Motivation

Enable expressing XMR's, particularly testing/verification scenarios, with minimal impact on the generated design other than requiring visibility of the probed elements. The goal is both testbench and design can be written in FIRRTL and have type-safe XMR's robust to hierarchy changes during compilation.

Second goal is to serve as pathway for producing resolved paths to signals for use in "raw" SV (e.g., testbench). Specifically, supporting the ability to use SV "force" on a probe (RWProbe<T>) is a requirement. These are typed differently as they cause much more conservative handling by the compiler.

References aren't Hardware, Cannot be "Connect"ed

To ensure a valid hierarchical reference can be emitted, the target must be below the access point in the hierarchy, although implementations are open to support other scenarios as they see fit / make sense in the target language.

References are not hardware types and as a result are not usable in most places -- they're only allowed as/in module ports. Additionally, similar to Analog, while they may be used in aggregates for convenience/grouping they cannot be used as part of "connect"s and so when mixed with hardware data, the hardware fields need to be connected explicitly and appropriate routing statements used for the reference (attach for Analog).

References are routed through the design statically, using new "forward" statement and originating "export" statement.

To support XMR'ing + separate compilation, extmodule may optionally specify resolved paths for its output references.

Example

An example demonstrating some features in the proposal, with possible output Verilog:

circuit Probe :
  module Child :
    input in : UInt<1>[2]
    output r : Probe<UInt<1>[2]>
    define r = probe(in)

  module Mid :
    input in : UInt<1>[2]
    output r : Probe<UInt<1>[2]>
    output r1 : Probe<UInt<1>>

    inst c of Child
    c.in <= in
    define r = c.r
    define r1 as c.r[1]

  module Probe :
    input in : UInt<1>[2]
    output r : Probe<UInt<1>[2]>
    output out : UInt<1>[2]
    output out_r1 : UInt<1>
    output out_r0_from_r : UInt<1>

    inst m of Mid
    m.in <= in
    define r = m.r

    out <= read(m.r)
    out_r1 <= read(m.r1)
    out_r0_from_r <= read(m.r[0])

Example output Verilog (no aggregate preservation, default):

module Child(
  input in_0,
        in_1
);

endmodule

module Mid(
  input in_0,
        in_1
);

  Child c (
    .in_0 (in_0),
    .in_1 (in_1)
  );
endmodule

module Probe(
  input  in_0,
         in_1,
  output out_0,
         out_1,
         out_r1,
         out_r0_from_r
);

  Mid m (
    .in_0 (in_0),
    .in_1 (in_1)
  );
  assign out_0 = Probe.m.c.in_0;
  assign out_1 = Probe.m.c.in_1;
  assign out_r1 = Probe.m.c.in_1;
  assign out_r0_from_r = Probe.m.c.in_0;
endmodule

See spec.md diff for additional FIRRTL examples.

Implementation and Related

FIRRTL References are inspired by and mostly map to CIRCT's firrtl.ref type and related operations (ref.send, ref.resolve) but the proposed references are designed to overcome some limitations with firrtl.ref and for better composition with existing FIRRTL / friendliness to generation.

CIRCT implementation of proposed references is underway.

[azidar]

Lots of questions, requests for additional examples, and some proposed syntax changes. Overall I really like the direction, thanks for all the hard work so far!

[azidar]

I'd recommend, rather than making the Probe's inner type passive, to actually change how output and input compose with a Probe. Basically, you can say that a flipped Probe type's port direction actually ignores the flip, rather than actually changing the type.

This enables the relative alignments of fields within the type of the thing you are probing to be preserved, which is probably an important thing to have access to where you are forcing/etc.

[dtzSiFive]

I'd recommend, rather than making the Probe's inner type passive, to actually change how output and input compose with a Probe. Basically, you can say that a flipped Probe type's port direction actually ignores the flip, rather than actually changing the type.

I may not entirely understand the suggestion here. Let's run with "Probes are still as proposed conceptually but they preserve the probed type (including flips) instead of making it passive" (where maybe your last sentence is indicating that we still treat it as if the inner was a passive type for purposes of initialization coverage and so on-- only the direction of the probe reference itself matters re:direction the reference is built up)?

If that's not it, please consider an example 😄 and/or we can schedule a chat 👍 (and report back here of course).

... are you're suggesting input/output indicators (and orientation of a fields, etc.) are ignored for portions of ports that contain/are of Reference type (leading /to/ the Probe, vs the type inside)?

Anyway. I think the second portion of this may be able to discuss at least somewhat orthogonally:

This enables the relative alignments of fields within the type of the thing you are probing to be preserved, which is probably an important thing to have access to where you are forcing/etc.

I'm not sure what you mean re:force, is there something there that motivates this that doesn't for a read?
Anyway, "what if everything was same except the Probe kept the flips in its inner type" was considered and rejected for a few reasons:

1. Encourages a fundamental misunderstanding of what probes are, specifically that they allow/support/are capable of having data flow opposite the direction the reference does (something like expecting input a: Probe<T>, output y: Probe<Y> to be at all similar to input Probe<{a : T, flip b: U}>). Generally it's important to understand an aggregate of probes (regardless of orientation) is not the same as a probe of an aggregate.

2. Passive type better captures the proposed semantics here: You're getting a read-only view of the probed element, that flows /together/ along the direction of the reference. This is true of other dataflow-y entities like "Node", same story here other than we make it easy to get a passive view of a non-passive bundle.

3. Only makes things harder to use: The primary motivation AFAIK for not making them passive would be, as I think you're suggesting, for ergonomics at the point of the access (perhaps more specifically: for parallelism in the way you interact with a resolved probe and if it was present locally). Unfortunately, since you cannot connect /to/ a read of a probe (only /from/) , this just means it's more painful to read back out as you have to manually connect each field of interest. Furthermore it's confusing as the probe() expression takes the entire mixed-orientation bundle as its argument, so if it's not passive it's especially confusing to have the data go the opposite way the type says it does. (also, see (2) again).

Let's continue discussing this 👍 .

[azidar]

Yeah let's sync up in a meeting and report back.

[dtzSiFive]

Summary of suggested alternative after discussion:

Push connect-all-regardless-of-alignment to read/force, Probe's have whatever type+alignment the originating target does. This pushes that behavior to read/force where in the current proposal this is done by the probe/rwprobe expression creating the Probe.

As I understand it, in nearly all cases this is semantically equivalent to the current proposal (only difference is that syntactically the Probe types have whatever alignment/full type the target does).
So it comes down to preference re:aesthetics/perspective I think and perhaps interaction with future ideas/directions (?). Without advocating one way or another, as an observation: in a future where we might have more powerful ways to project/view/connect bundles the difference may be more significant/relevant.

One phrasing of the difference here:
is it nicer for inner type to always match the probe target (even if that doesn't help/change anything), or should the inner type reflect the type of the probed data you interact with (always passive, not necessarily the same as type of the target)? If/when we support references to circuit elements directly (not probes of their data) I think those types should definitely match the originating type (something like: Wire<{a : UInt, flip b: UInt}> or more usefully a Memory, Instance, or Module).

One result of keeping the alignment would be that now the probes for two entities that have identical types other than alignment are no longer equivalent, even though they describe the same data and are interacted with in the same way (FWIW). While we could say they're compatible, I think if that's the case we shouldn't bother recording the flip-ness anyway. I don't think this is especially important, but would be one difference.

[azidar]

Excellent summary. I think the main advantage to me is that you are preserving as much type information as possible, for as long as possible. Since reference types will be used at the interface level, I think including that information seems useful for future purposes. For example, I could imagine a "force only the flipped fields" or "force only the aligned fields" as an API a user may want to support in the future in Chisel, which would require preserving the relative field alignments. E.g. using effectively Chisel's :<= operator, but with forces. Semantically, this would be to "force only the data but not the backpressure fields".

In order to do this, you'd need the ability to select the right fields, which would be impossible if the type was converted to being passive.

[seldridge]

This is looking fantastic. Thanks for doing some deep thinking on all the angles to this thorny issue. Some comments through, but nothing blocking.

[seldridge]

Is it mentioned anywhere above that probes cannot be constant?

[dtzSiFive]

Yes, should clarify that a ref is already const so no reason to mark it as such. And that cannot get a RWProbe to a const thing.

I'm making a note to do another pass with const in mind, now that this has come up. It was merged right as I was submitting the PR so the integration of the spec language may have been a little hasty, thanks/apologies. However I think this is right (but should be made clear so you don't have to gather it by inspecting the grammar! 😆):

References are always "const", arguably "more const than const" in that they /must/ be known statically (not only that they cannot mutate at "runtime"). I suppose we could allow someone to redundantly mark them "const" (similarly, we may choose to accept "const const UInt" as harmless), don't feel strongly about that at all.

References /to/ const are supported, and as mentioned above I should clarify you cannot get a RWProbe to anything const! Thanks 😄.

[mwachs5]

should clarify you cannot get a RWProbe to anything const!

hm, interesting. Why not. If I have a Const input port, why not let the force mechanism be the one to set it.

[darthscsi]

@dtzSiFive and I discussed this some. On one hand, forcing a constant seems to be non-constanty. On the other hand, verification is probably going to want to do that anyway.

[azidar]

I'm actually not convinced of that. The common use-case for internal forcing for verification is to fast-forward a simulation to get to a state that you then want to check; in this scenario, you would only be forcing signals that wouldn't be constant. Can we make it illegal to force const for now, and then make it legal once we have a concrete use-case?

[mwachs5]

have we defined public modules in an earlier PR? If not, maybe we should make sure to do that and point to the formatl definition.

[dtzSiFive]

Not yet, filed an issue since it seems this is something we're wanting to add/refer to: #82

[mwachs5]

somehow I had a few comments in Pending state

[mwachs5]

should clarify you cannot get a RWProbe to anything const!

hm, interesting. Why not. If I have a Const input port, why not let the force mechanism be the one to set it.

[dtzSiFive]

Responding to one comment here, since I can't seem to figure out how to respond more directly to it:

hm, interesting. Why not. If I have a Const input port, why not let the force mechanism be the one to set it.

Interesting! The original explanation was because force is used to change drivers (that already exist, or it's an invalid circuit) and that seems not what being "const" is about, unless the values are the same but then the force is not doing anything.

Thinking on it a bit further-- if we dodge the question of how we get a const thing that doesn't have a driver already, in theory force could be useful for driving a port, and the "const" modifier is the user's way or promising all observations of the const entity will see the same value, so maybe could work and be useful.

This runs into questions of initialization order that I don't know the answer to but am a little allergic to introducing too eagerly into FIRRTL semantics (in other languages this can be a right mess) -- for example, when exactly do initial blocks run with respect to other initial blocks or execution of remote modules? We would need to ensure the force was "live" before any use of the const port, otherwise it could be observed to have different values at different points of the execution. And of course hopefully users don't force it to something different later, or use release, but that arguably would be a user error.

All that said, we could also support RWProbe of a const in the spirit of "this seems dangerous but don't want to get in the way", and let the user who promises "const"-ness take full responsibility for such concerns, as they somewhat already are obligated to do. (e.g., a testbench author who fully controls initialization story already could ensure whatever is needed-- since all const ports already need to tackle such initialization ordering concerns, I think).

Thanks for the question, inclined to disallow for immediate future for the above reasons but this may have more legs than I thought at a glance.

[ekiwi]

Do you mean that they cannot be represented directly in the target language?
I assume the idea is to lower them to something that can be represented, correct?
To me the sentence currently suggests that they are just ignored, which would defeat their purpose.

[ekiwi]

I am probably misunderstanding this a little and I would appreciate your clarification:

What is the point of reference types, when you need to explicitly wire them through the hierarchy? Why not just add the force and release construct and let it point to any arbitrary signal in the hierarchy without explicitly forwarding and wiring things through the module hierarchy? Like in SystemVerilog, if I want to bind to a signal in a module, I do not need to modify the code of that module, here it seems like I would have to change the source code to explicitly expose signals and even change all the ports from the module I am trying to inspect to where I need to see the signal value, which seems to be about the same amount of work as just adding a regular port. What am I missing?

[dtzSiFive]

I am probably misunderstanding this a little and I would appreciate your clarification:

What is the point of reference types, when you need to explicitly wire them through the hierarchy? Why not just add the force and release construct and let it point to any arbitrary signal in the hierarchy without explicitly forwarding and wiring things through the module hierarchy? Like in SystemVerilog, if I want to bind to a signal in a module, I do not need to modify the code of that module, here it seems like I would have to change the source code to explicitly expose signals and even change all the ports from the module I am trying to inspect to where I need to see the signal value, which seems to be about the same amount of work as just adding a regular port. What am I missing?

Great question, thanks!

The primary motivation here is to expose signals in a way that FIRRTL code can be written to use them in a way that:

1. Doesn't require knowledge of the path to the signal, just that it exists (and perhaps is part of a relevant bundle)

• Not important what the path is just that one exists to that signal in the final design, regardless of optimizations / hierarchy changes

2. All hierarchical reference targets are known in the module so they can be preserved or emitted appropriately (don't optimize away a signal that may be read, don't optimize through a signal that may be forced, see [major] Add reference types. #75 (comment) for other reasons force-able is important for the compiler to know about).
   Making references an explicit part of a module's "interface" is a key part of the motivation here.

One goal is to have minimal impact on the produced design, other than the considerations in (2) above. This is why while these references are part of the FIRRTL module signature they do not become Verilog ports -- idea being that if we wanted ports these should just be wires 😄 .

Compared to putting the path in the XMR operations, this provides a bit more explicit type-safety and doesn't require local knowledge of the access path to the signal.

You're right, this is about the same work as wiring a port out of the design.

The approach proposed here perhaps makes more sense (e.g., the type-safety could be checked by chasing down the target and using that type, etc.) when considering using a FIRRTL module w/references without knowledge of or easy ability to know the paths involved at the point of generating the XMR operations. Either by dropping a general testbench into multiple designs where the signals might be located in different places but are exposed under the same interface/signature for consumption, or especially by using references from an extmodule implemented by another FIRRTL design / "circuit".

Ideally the latter scenario would be written without the ref statements including the path when the extmodule is also written in FIRRTL, but until we have a stronger story about compiling FIRRTL bits separately and integrating them, these are the means for adding the requisite metadata for the XMR's to be emitted. This also is the mechanism for XMR'ing into modules not written in FIRRTL.

[ekiwi]

Thanks for your great explanation, @dtzSiFive! I see that this does not really replace BoringUtils, but is more of a verification mechanism where you want to explicitly expose signals in your design.

I still feel like the verification signals should be declared separately from the actual RTL implementation, but I can see how explicitly encoding this into the design can have its upsides.

Maybe one future extension could allow you to skip the explicit forwarding when desired by referring to things deeper in the hierarchy directly. Then the compiler can add the missing ports and forwarding statements.

[dtzSiFive]

Thanks for your great explanation, @dtzSiFive! I see that this does not really replace BoringUtils, but is more of a verification mechanism where you want to explicitly expose signals in your design.

That's 100% correct, and you're welcome. Glad the explanation was useful!

Note that force and release are not usually synthesizable AFAIK, and so are more suited for verification/simulation-only uses already.

Maybe one future extension could allow you to skip the explicit forwarding when desired by referring to things deeper in the hierarchy directly. Then the compiler can add the missing ports and forwarding statements.

Interesting! I think that would be useful but have reservations re:making sure that works nicely. Moving supported features out of annotations is always nice 👍, especially if that involves giving them carefully considered and official semantics. Something like this would allow moving away from BoringUtils for some use cases. Note that BoringUtils adds real ports (and results in different RTL of course), and furthermore in theory can be used to plumb signals up, down, wherever (not true of references; and presently has issues re:plumbing out/in when blocks, see llvm/circt#4332 ).
I am not sure if this is where we want to go, but I think you're not the only one who'd like to see BoringUtils more officially supported in some capacity, so if you're interested perhaps we should start a discussion issue for this separately?

For BoringUtils and possibly auto-plumbing references, I think it's important for "public" modules to have a known interface. The existence of a higher-up module wanting a signal causing a lower module to change its signature might not be ideal. As we work out how separate compilation might work, these techniques are unlikely to make sense across compilation boundaries, but could still be useful. Anyway, a discussion on that sounds good to me 👍 .

[seldridge]

I see that this does not really replace BoringUtils, but is more of a verification mechanism where you want to explicitly expose signals in your design.

Right---this isn't BoringUtils. There is, however, a Chisel API that should be added, e.g., Tap or Force, which has a similar behavior as BoringUtils except that it creates reference types.

It is important to decouple the API of BoringUtils/Tap from the Chisel-side implementation of them. Both of these should create all ports (real or reference type) in the FIRRTL they emit. A FIRRTL Compiler should not be responsible for doing wiring. This is currently difficult in Chisel due to restrictions on module modification after module close. This should all be fixed as Chisel evolves.

Maybe one future extension could allow you to skip the explicit forwarding when desired by referring to things deeper in the hierarchy directly. Then the compiler can add the missing ports and forwarding statements.

This is totally reasonable for a Chisel API (which is essentially what BoringUtils-style APIs should provide). A user shouldn't be responsible for doing manual wiring. However, this isn't a great property for an IR: (1) it's expensive to check, (2) it can error in strange ways, and (3) it's difficult to transform.

[ekiwi]

Note that force and release are not usually synthesizable AFAIK, and so are more suited for verification/simulation-only uses already.

I feel like the references described in the PR could totally be lowered to hardware, with the one challenging exception being memory references.

The existence of a higher-up module wanting a signal causing a lower module to change its signature might not be ideal. As we work out how separate compilation might work, these techniques are unlikely to make sense across compilation boundaries, but could still be useful.

The signature would only be changed in the output of the compiler, not in the original Chisel sources. So it essentially be "source compatible" but not "binary compatible". I am curious to see where your work on compilation boundaries goes since - afaik - SystemVerilog does not really have that concept as bind allows you to hook into internal signals and users are generally used to being able to force any signal when running a simulator. Thus I wonder how much a more strict separation in firrtl can actually improve the speed of the end-to-end flow.

[ekiwi]

A FIRRTL Compiler should not be responsible for doing wiring.

I don't quite understand why a FIRRTL compiler should not be doing wiring. Isn't the whole point of FIRRTL to simplify the Chisel frontend and rely on a compiler to "desugar" more complicated concepts like arbitrary cross module references?

However, this isn't a great property for an IR: (1) it's expensive to check, (2) it can error in strange ways, and (3) it's difficult to transform.

I am not sure why this would be the case. If you allow something like creating a ref to x.childA.childB.y you go down the hierarchy and add the ports as appropriate. If a child or the final signal does not exist, you just throw that as an error message. Such an error would be pretty unlikely anyways since the signal must have existed in the original Chisel. This does sound like a fairly straight forward transform to me on the FIRRTL IR, whereas auto-adding ports in the Chisel frontend seems quite hard to me.

[azidar]

Love the conversation about auto-punching, but can we move it to a dedicated issue? #81

[darthscsi]

LGTM

[darthscsi]

@dtzSiFive and I discussed this some. On one hand, forcing a constant seems to be non-constanty. On the other hand, verification is probably going to want to do that anyway.

[seldridge]

This looks excellent. Thanks for all your hard work on this @dtzSiFive.

stale, most comments addressed, thanks will

[seldridge]

Small changes in late commits LGTM.

[seldridge]

It makes a lot of sense to me to restrict this. 👍

[azidar]

Looks great!!! My one opinion that I still hold somewhat strongly is that the inner types of a Probe should contain flips, but that the force/release/read should ignore flips. However, its something we can change later if necessary.