[FIRRTL] 'Conditional' Analog attach
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
I've been messing around with FIRRTL attach operations nested in `WhenOp` regions fully expecting them to be illegal. After all Chisel refuses to produce them. But, after careful reading of the spec, it appears to me that they should simply be unaffected by the condition and, in fact, this is what both SFC and firtool do:
foo.fir:
```
circuit Example :
module Example :
output a : Analog<1>
output b : Analog<1>
output c : Analog<1>
output d : Analog<1>
when UInt<1>("h1") :
attach (a, b)
else :
attach (c, d)
```
```
> firtool --verilog /tmp/foo.fir
// Generated by CIRCT unknown git version
module Example(
inout a,
b,
c,
d
);
`ifdef SYNTHESIS
assign a = b;
assign b = a;
assign c = d;
assign d = c;
`else // SYNTHESIS
`ifdef verilator
`error "Verilator does not support alias and thus cannot arbitrarily connect bidirectional wires and ports"
`error "Verilator does not support alias and thus cannot arbitrarily connect bidirectional wires and ports"
`else // verilator
alias a = b;
alias c = d;
`endif // verilator
`endif // SYNTHESIS
endmodule
```
But if we run the canonicalizer added in #6236 before the `WhenOp` is removed by ExpandWhens, the 'unreachable' attach is dropped too:
```
> .firtool --parse-only /tmp/foo.fir | circt-opt --canonicalize | firtool -format=mlir --verilog
// Generated by CIRCT unknown git version
module Example( // :3:5
inout a, // :3:32
b, // :3:59
c, // :3:86
d // :3:113
);
`ifdef SYNTHESIS // :4:7
assign a = b; // :4:7
assign b = a; // :4:7
`else // SYNTHESIS
`ifdef verilator // :4:7
`error "Verilator does not support alias and thus cannot arbitrarily connect bidirectional wires and ports" // :4:7
`else // verilator
alias a = b; // :4:7
`endif // verilator
`endif // SYNTHESIS
endmodule
```
This is obviously a highly constructed example, but it did cause me some confusion. I have no clue under what circumstances we expect canonicalization to be run before ExpandWhens. But since it was added there appears to be a use case and I guess this technically violates the spec?
Contributor guide
No contributing guide indexed for this repository
Research direction
Reproduce the example with firtool --parse-only, circt-opt --canonicalize, and firtool, comparing it with direct firtool output. Start by examining the canonicalizer introduced in #6236 and the ExpandWhens handling of attach operations. Done means the behavior of conditional analog attaches is made consistent with the FIRRTL specification and the observed tool behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 32/100