chipsalliance / firrtl-spec

The specification for the FIRRTL language
44 stars 28 forks source link

[patch] Rewrite Types Section #145

Closed mmaloney-sf closed 11 months ago

mmaloney-sf commented 11 months ago

This PR is a moderate rewrite of the entire Types section.

This is best reviewed by building the PDF and reviewing paragraph by paragraph.

Here are some highlights:

mmaloney-sf commented 11 months ago

@darthscsi I rewrote the Constant Types section as best as I could.

Being such a massively cross-cutting feature, it's hard to spec out in just one section. I think the rewritten version touches on all points from the original:

Thank you @seldridge for letting me bounce my thoughts off of you.

mmaloney-sf commented 11 months ago

One big addition at the last minute:

I define a Connectable Type as a type which may appear as the type of an expression which may participate in a connect statement.

Connectable types appear internally in the FIRRTL MLIR dialect. They affect how components are split up when lowering to Verilog.

Currently, I only rewrote the enumerations section in terms of these. (Namely, the types of variants must be connectable and passive). But as I get to the section on Connections, we can apply this definition cleanly there.

This definition will also help us untangle some thorny issues in FIRRTL, which add a disproportionate amount of complexity to the spec:

mmaloney-sf commented 11 months ago

I forgot -- I'm a bit stuck on the section on Probes and Type Inference.

@dtzSiFive Can you advise me on how to handle this section?

mmaloney-sf commented 11 months ago

Just to clarify since @seldridge had left a number of comments I closed without addressing:

The section on Type Inference and the section on Type Equivalence were left mostly unchanged. Both require a lot of care to reword without changing the meaning. Furthermore, the latter is acutely underspecified.

I think all of the comments I closed without fixing are worthy of being addressed. I am just postponing that work for a separate PR, so it doesn't get lost while picking all the low-hanging fruit.