[FIRRTL][Inliner] Hierpath's used by non-NLA's are not handled well
- Dominant language
- C++
- Stars
- 2.2k
- Forks
- 524
- Avg merge
- 3d 2h
- Merged PRs (30d)
- 46
Description
When performing its transformations over the hierarchy, our inliner goes to some lengths to manage annotations and the hierpath's they reference.
Consider this input, which we'll use to demonstrate two problems:
``` mlir
firrtl.circuit "XMRRef" {
hw.hierpath private @xmr [@Foo::@bar, @Bar::@b]
firrtl.module private @Bar() {
%b = firrtl.wire sym @b : !firrtl.uint<1>
}
firrtl.module private @Foo() {
firrtl.instance bar sym @bar @Bar()
}
firrtl.module @XMRRef() attributes {annotations = [{class = "firrtl.transforms.FlattenAnnotation"}]} {
firrtl.instance foo sym @foo @Foo()
firrtl.instance foo sym @foo2 @Foo()
// Before inlining, this is a single path, after it's two!
%xmr = sv.xmr.ref @xmr : !hw.inout
}
}
```
1. Inliner happily deletes hierpath's if they would have made an NLA with that hierpath "dead". Latest `circt-opt --firrtl-inliner` yields invalid IR:
```
xmrref.mlir:14:12: error: Referenced path doesn't exist @xmr
%xmr = sv.xmr.ref @xmr : !hw.inout
^
xmrref.mlir:14:12: note: see current operation: %2 = "sv.xmr.ref"() <{ref = @xmr, verbatimSuffix = ""}> : () -> !hw.inout
```
2. As indicated in the comment within the MLIR snippet above, this is fundamentally intractable. Unlike annotations which we can clone, remove, or "make local", other hierpath uses by other operations simply cannot ("as-is") be freely duplicated, removed, so on.
In particular, the inlining transformation on the provided IR simply cannot work: there is no XMR path that will name all copies of the named wire.
-------------
Dropping the `sv.xmr.ref` operation, here's the output of the inliner:
``` mlir
firrtl.circuit "XMRRef" {
firrtl.module @XMRRef() {
%foo_bar_b = firrtl.wire sym @b : !firrtl.uint<1>
%foo_bar_b_0 = firrtl.wire sym @b_0 {name = "foo_bar_b"} : !firrtl.uint<1>
}
}
```
This example intentionally ensures the original path doesn't exist anymore (having its target inlined twice, cloning it to two targets), but things get trickier in a way if we, for example, change `@Foo` to not be private, we get the following IR:
```mlir
firrtl.circuit "XMRRef" {
hw.hierpath private @xmr [@Foo::@bar, @Bar::@b]
firrtl.module private @Bar() {
%b = firrtl.wire sym @b : !firrtl.uint<1>
}
firrtl.module @Foo() {
firrtl.instance bar sym @bar @Bar()
}
firrtl.module @XMRRef() {
%foo_bar_b = firrtl.wire sym @b : !firrtl.uint<1>
%foo_bar_b_0 = firrtl.wire sym @b_0 {name = "foo_bar_b"} : !firrtl.uint<1>
%0 = sv.xmr.ref @xmr : !hw.inout
}
}
```
Now the XMR references something from another hierarchy / top.
Imagine this operation was writing through the XMR to drive the target; now the wires are undriven in that way when instantiated in the primary hierarchy. Or emitted into metadata via a verbatim or other use.
Regardless of how compelling you find this specific example, the issue is that we have fundamentally altered the /functionality/ of the circuit in observable ways **silently**.
Contributor guide
No contributing guide indexed for this repository
Research direction
Reproduce the examples with circt-opt --firrtl-inliner and inspect the FIRRTL inliner handling of hw.hierpath and sv.xmr.ref. The work is complete only when inlining does not leave dangling hierpath references or silently alter observable circuit behavior; the issue does not name a source file or test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100