chipsalliance / firrtl-spec

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

[major] Define option groups and instance choices #155

Open nandor opened 10 months ago

nandor commented 10 months ago

Add option and instance choice features to the FIRRTL spec. Define an ABI for these that uses files similar to how layers work.

There are more examples of syntax and possible ABIs in this repository: https://github.com/seldridge/instance-choice-abis

More Information

This feature exists to serve two separate purposes both related to allowing for limited parameterization of a FIRRTL circuit. This parameterization is later removed via specialization either: (1) by a FIRRTL compiler or (2) via Verilog elaboration.

Targets describe available paramterization as well as legal options for that target. (Names subject to change.) These target can then be used by FIRRTL constructs to describe behavior which is dependent on the value of a target.

Currently, only one construct can depend on the value of a target: instance choice. An instance choice is an instantiation of one of several modules where the choice of instantiation is controlled by the option selected for a given target. E.g., a likely target is one which captures "FPGA or ASIC or Generic" that, when set, can specialize the design for an FPGA, and ASIC, or give generic Verilog. E.g., a more "parameter-like" target would be one that sets the cache size.

All modules instantiated by an instance choice must have the exact same port-level interface including probes and properties. This is necessary to allow them to be substituted. The underlying hardware backing the probes and properties may vary from one instance choice to another. This is intentional and critical for this to be useful. Properties require no special treatment other than a specialization must be chosen before properties can be evaluated. Probes require tedious Verilog macros that allow the Verilog-level specialization to set the internal path.

mmaloney-sf commented 10 months ago

I am concerned about the orthogonality of this feature with external modules and intrinsic modules. It's hard for me to tell how this functionality is different in intent from those two features.

I also get a feeling this feature is overfitted to one particular usecase. Let's make sure we're being especially mindful of the complexity budget in FIRRTL. A feature like this feels to me like it might be better captured at the Chisel level. Are there arguments otherwise?

More detailed review is incoming.

nandor commented 10 months ago

I think we had a lengthy discussion about the need for this feature a while back, which I do not want to repeat here. External modules and intrinsic modules do not allow for the sort of optionality provided by this feature: we need to let users specialise for targets, FPGA and ASIC especially, without involving compiler-generated intrinsics or resolving the differences later via extmodules, out of the reach of the compiler.

It is not sufficient to capture this information at the Chisel generator level since then we can generate a design for only one of the options, we cannot encapsulate multiple possible targets as layers or this feature allow. At the Chisel level decisions can only be made if information is propagated through to the instance site, something that is not always possible, particularly in the case of the tool we are building this for. This warrants a feature which makes the decision based on a global flag somewhere late in the pipeline.

seldridge commented 10 months ago

@rwy7: FYI, give this some thought about composition with true Verilog width parameterization.

nandor commented 10 months ago

I pushed a rewrite. I would want to focus on the syntax (as I am fairly open to changes) and I want to settle the debate about extmodules. Implementation-wise, an op in the FIRRTL dialect is necessary, but I can move forward with that.

I am approaching this problem from the perspective of algebraic data types/sum types, hence the terminology.

seldridge commented 4 months ago

After some discussion with @rwy7, option seems like a better name to describe what is going on. I'll revise this and get this moving.