-
Notifications
You must be signed in to change notification settings - Fork 31
[minor] Add Instance Choice #265
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -311,6 +311,136 @@ bind Bar Bar_Layer1_Layer2 layer1_layer2(.notA(Bar.layer1.notA)); | |
| The `` `ifdef ``{.verilog} guards enable any combination of the bind files to be included while still producing legal SystemVerilog. | ||
| I.e., the end user may safely include none, either, or both of the bindings files. | ||
|
|
||
| ## On Targets | ||
|
|
||
| A target will result in the creation of specialization files. | ||
| One specialization file per public module per option for a given target will be created. | ||
|
|
||
| Including a specialization file in the elaboration of Verilog produced by a FIRRTL compiler specializes the instantiation hierarchy under the associated public module with that option. | ||
|
|
||
| Each specialization file must have the following filename where `module` is the name of the public module, `target` is the name of the target, and `option` is the name of the option: | ||
|
|
||
| ``` ebnf | ||
| filename = "targets_" , module , "_" , target , "_", option , ".vh" ; | ||
| ``` | ||
|
|
||
| Each specialization file will guard for multiple inclusion. | ||
| Namely, if two files that specialize the same target are included, this must produce an error. | ||
| If the file is included after a module which relies on the specialization, this must produce an error. | ||
|
|
||
| ### Example | ||
|
|
||
| The following circuit is an example implementation of how a target can be lowered to align with the ABI defined above. | ||
| This circuit has one target named `Target` with two options, `A` and `B`. | ||
| Module `Foo` may be specialized using an instance choice that will instantiate `Bar` by default and `Baz` if `Target` is set to `A`. | ||
| If `Target` is set to `B`, then the default instantiation, `Bar`, will occur. | ||
| Module `Foo` includes a probe whose define is inside the instance choice. | ||
|
|
||
| ``` firrtl | ||
| FIRRTL version 4.1.0 | ||
| circuit Foo: | ||
|
|
||
| option Target: | ||
| case A | ||
| case B | ||
|
|
||
| module Baz: | ||
| output a: Probe<UInt<1>> | ||
|
|
||
| node c = UInt<1>(1) | ||
| define a = probe(c) | ||
|
|
||
| module Bar: | ||
| output a: Probe<UInt<1>> | ||
|
|
||
| node b = UInt<1>(0) | ||
| define a = probe(b) | ||
|
|
||
| public module Foo: | ||
| output a: Probe<UInt<1>> | ||
|
|
||
| instchoice x of Bar, Target: | ||
| A => Baz | ||
|
|
||
| define a = x.a | ||
| ``` | ||
|
|
||
| To align with the ABI, this must produce the following files to specialize the circuit for option `A` or option `B`, respectively: | ||
|
|
||
| - `targets_Foo_Target_A.sv` | ||
| - `targets_Foo_Target_B.sv` | ||
|
|
||
|
Comment on lines
+368
to
+372
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may need to generate a header for default target as well since there would be a macro dependency from
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, having a default is fine. That would need to have some filename that can't collide with the other options to make it work. Not a problem. Thank you for thinking about this deeply!
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think i have three options:
|
||
| What follows describes a possible implementation that aligns with the ABI. | ||
| Note that the internal details are not part of the ABI. | ||
|
|
||
| When compiled, this produces the following Verilog: | ||
|
|
||
| ``` systemverilog | ||
| module Baz(output a); | ||
| assign a = 1'h1; | ||
| endmodule | ||
|
|
||
| module Bar(output a); | ||
| assign a = 1'h0; | ||
| endmodule | ||
|
|
||
| // Defines for the instance choices | ||
| `ifndef __target_Target_foo_x | ||
| `define __target_Target_foo_x Bar | ||
| `endif | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment for link. |
||
|
|
||
| module Foo(); | ||
|
|
||
| `__target_Target_foo_x x(); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need this macro replacement for module name (which requires a additional very specialized operation to HW)? Can we do that with a plain if-def chains? |
||
|
|
||
| endmodule | ||
| ``` | ||
|
|
||
| The contents of the two option enabling files are shown below: | ||
|
|
||
| ``` systemverilog | ||
| // Contents of "targets_Target_A.vh" | ||
| `ifdef __target_Target_foo_x | ||
| `ERROR__target_Target_foo_x__must__not__be__set | ||
| `else | ||
| `define __target_Target_foo_x Baz | ||
| `endif | ||
|
|
||
| `ifdef __targetref_Foo_x_a a | ||
| `ERROR__targetref_Foo_x_a__must__not__be__set | ||
| `else | ||
| `define __targetref_Foo_x_a a | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note that we cannot do emit FIRRTL version 5.1.0
circuit Top :
option Platform :
FPGA
option Optimization :
Fast
module DefaultTarget :
output probe : Probe<UInt<8>>
wire r : UInt<8>
connect r, UInt<8>(0)
define probe = probe(r)
module FastTarget :
output probe : Probe<UInt<8>>
wire w : UInt<8>
connect w, UInt<8>(1)
define probe = probe(w)
module FPGATarget :
output probe : Probe<UInt<8>>
instchoice opt_inst of DefaultTarget, Optimization:
Fast => FastTarget
define probe = opt_inst.probe
public module Top :
output probe: Probe<UInt<8>>
instchoice platform_inst of DefaultTarget, Platform:
FPGA => FPGATarget
define probe = platform_inst.probe
// in targets_Platform_FPGA.svh
`define __targetref_Top_platform_inst_probe `__target_Platform_Top_platform_inst.`__targetref_FPGATarget_opt_inst_probe
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice find! Can we break the file inclusion problem by requiring the ordering of the files to be:
Since we've backtracked on the need to hide all instance choices, and instead can handle that with a dedicated tool, this might work. It does mean that the ref header is going to be complex and likely have to mirror the instantiation hierarchy and the options along the way. Alternatively, (1) could include partial ref paths which are assembled into the final paths in (2).
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
That is close to what I've implemented so far (= simply inlining paths in The issue with this approach it's necessary to hide specific options/probes from ref files, but yes, it's possible to do with post-process. |
||
| `endif | ||
| ``` | ||
|
|
||
| ``` systemverilog | ||
| // Contents of "targets_Target_B.vh" | ||
| `ifdef __target_Target_foo_x | ||
| `ERROR__target_Target_foo_x__must__not__be__set | ||
| `endif | ||
|
|
||
| // This file has no defines. | ||
| ``` | ||
|
|
||
| Additionally, probe on public module `Foo` requires that the following file is produced: | ||
|
|
||
| ``` systemverilog | ||
| // Contents of "refs_Foo.sv" | ||
| `ifndef __targetref_Foo_x_a | ||
| `define __targetref_Foo_x_a b | ||
| `endif | ||
|
|
||
| `define ref_Foo_x x.`__targetref_Foo_x_a | ||
| ``` | ||
|
|
||
| If neither of the option enabling files are included, then `Bar` will by default be instantiated. | ||
| If `targets_Foo_Target_A.vh` is included before elaboration of `Foo`, then `Baz` will be instantiated. | ||
| If `targets_Foo_Target_B.vh` is included before elaboration of `Foo`, then `Bar` will be instantiated. | ||
| If both `targets_Foo_Target_A.vh` and `targets_Foo_Target_B.vh` are included, then an error (by means of an undefined macro error) will be produced. | ||
| If either `targets_Foo_Target_A.vh` or `targets_Foo_Target_B.vh` are included after `Foo` is elaborated, then an error will be produced. | ||
|
|
||
| If `ref_Foo.vh` is included before either `targets_Foo_Target_A.vh` or `targets_Foo_Target_B.vh`, then an error will be produced. | ||
|
|
||
| ## On Types | ||
|
|
||
| Types are only guaranteed to follow this lowering when the Verilog type is on an element which is part of the ABI defined public elements. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Switch to
-delimiters to avoid conflicts with FIRRTL identifiers. This is the same trick we played with layer files.