[ExportVerilog] Try to make bind change the generated RTL as little as possible
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
# Background
We would like generated RTL to be as invariant to bind statements as possible[^1].
For example, given the following original IR:
```mlir
hw.module @Bottom(%I: !hw.struct) -> (O: !hw.struct) {
hw.output %I : !hw.struct
}
hw.module @Middle(%I: !hw.struct) -> (O: !hw.struct) {
%0 = hw.instance "bottom" @Bottom(I: %I: !hw.struct) -> (O: !hw.struct)
hw.output %0 : !hw.struct
}
hw.module @Top(%I: !hw.struct) -> (O: !hw.struct) {
%0 = hw.instance "middle" @Middle(I: %I: !hw.struct) -> (O: !hw.struct)
hw.output %0 : !hw.struct
}
```
if we add a few lines to do a bind (including some XMR statements):
```mlir
hw.module @TopXMRAsserts(%I: !hw.struct, %O: !hw.struct, %a: !hw.struct, %b: i1) -> () {
%0 = hw.struct_extract %I["x"] : !hw.struct
%1 = hw.struct_extract %I["y"] : !hw.struct
%2 = hw.struct_extract %O["x"] : !hw.struct
%3 = hw.struct_extract %O["y"] : !hw.struct
%4 = hw.struct_extract %a["x"] : !hw.struct
%5 = hw.struct_extract %a["y"] : !hw.struct
}
hw.module @Bottom(%I: !hw.struct) -> (O: !hw.struct) {
hw.output %I : !hw.struct
}
hw.module @Middle(%I: !hw.struct) -> (O: !hw.struct) {
%0 = hw.instance "bottom" @Bottom(I: %I: !hw.struct) -> (O: !hw.struct)
hw.output %0 : !hw.struct
}
hw.module @Top(%I: !hw.struct) -> (O: !hw.struct) {
%0 = hw.instance "middle" @Middle(I: %I: !hw.struct) -> (O: !hw.struct)
%2 = sv.xmr "middle", "bottom", "O" : !hw.inout>
%1 = sv.read_inout %2 : !hw.inout>
%4 = sv.xmr "middle", "bottom", "I.x" : !hw.inout
%3 = sv.read_inout %4 : !hw.inout
hw.instance "TopXMRAsserts_inst0" sym @Top.TopXMRAsserts_inst0 @TopXMRAsserts(I: %I: !hw.struct, O: %0: !hw.struct, a: %1: !hw.struct, b: %3: i1) -> () {doNotPrint = 1}
hw.output %0 : !hw.struct
}
sv.bind #hw.innerNameRef<@Top::@Top.TopXMRAsserts_inst0>
```
then we would *ideally* like the generated Verilog to go from
```verilog
// Generated by CIRCT circtorg-0.0.0-658-g0d82b4bb2
module Bottom(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
assign O = I;
endmodule
module Middle(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
Bottom bottom (
.I (I),
.O (O)
);
endmodule
module Top(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
Middle middle (
.I (I),
.O (O)
);
endmodule
```
to
```verilog
// Generated by CIRCT circtorg-0.0.0-658-g0d82b4bb2
module Bottom(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
assign O = I;
endmodule
module Middle(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
Bottom bottom (
.I (I),
.O (O)
);
endmodule
module TopXMRAsserts(
input struct packed {logic x; logic y; } I,
O,
a,
input b);
endmodule
module Top(
input struct packed {logic x; logic y; } I,
output struct packed {logic x; logic y; } O);
Middle middle (
.I (I),
.O (O)
);
endmodule
// ----- 8< ----- FILE "bindfile" ----- 8< -----
bind Top TopXMRAsserts TopXMRAsserts_inst0 (
.I (I),
.O (O),
.a (middle.bottom.O),
.b (middle.bottom.I.x)
);
```
Note that the only diff between the two Verilog's is the inclusion of the new `module TopXMRAsserts` and the `bind` statement at the end of the file. The internals of all other modules are unchanged.
[^1]: The motivation for this is that often production flows have the notion of "frozen" RTL, where functional/logical RTL is not allowed to change, but verification collateral (such as bound assertions) are allowed to be added. When the Verilog (incl. assertions) is generated from higher level tools (e.g. magma, Chisel), we need to ensure that the RTL does not change.
# Assumptions
We certainly can't guarantee that any arbitrary bind statement does not change the generated Verilog. Take the simple example of passing a newly created value (not otherwise used) into a bound instance:
```mlir
%new_value = comb.add %x, %y : i8
hw.instance ... (%new_value) ...
```
Therefore we first consider the following assumptions:
* Bound instances only have input ports, i.e. they have neither have output ports nor inout ports (this may already be an assumption...)
* All drivers of bound instance input ports are either:
* Ports of the containing module[^2]
* Ports of other (hw) instances contained in the contained module (footnote 2 also applies here)
* Reads of XMR values
[^2]: When we say that all drivers are a port, we may also want to additionally allow derivations of said port, e.g. `hw.array_select`, `hw.struct_extract`, or `comb.extract`. This may make this change harder, in which case we may want to reconsider the assumptions. For example, we may convert all such cases into XMR reads so as to avoid this case.
# Proposal
Given the above assumptions I propose we do the following:
1. Avoid spilling any wires for XMR reads inside of bound instances
2. Directly emit the XMR path for XMR's driving instance input ports (perhaps this can be regardless of whether or not the instance is bound -- or we can do it only for bound instances)
3. If a bound instance port driver refers to a port of the containing module, or a port of an instance in the containing module, then we
(a) avoid spilling a wire (unless otherwise already spilled)
(b) emit the port name directly in the bind statement
# Extras
Per the original motivation, we would also like the following minor changes
* Add an option to not emit the `This instance is elsewhere emitted...` comment (or skip it altogether).
cc @uenoku
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the ExportVerilog entry point and its handling of bound instances, XMR reads, and emitted wires. Compare generated output with and without the described bind under the stated assumptions; done means the existing modules remain unchanged while the bound module and bind statement are added.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100