llvm / llvm/circt

[FIRRTL][LowerLayers] Lower layers does not update HPs of non-local annotations

Open
#7,144 0 comments 0 reactions 1 assignee View on GitHub

@rwy7 is already working on this.

Since Jun 7, 2024.

FIRRTL
Dominant language
C++
Stars
2.2k
Forks
524
Avg merge
3d 2h
Merged PRs (30d)
46

Description

Given this firrtl IR:
```firrtl
FIRRTL version 4.0.0
circuit Top: %[[ {"class": "circt.test", "target": "~Top|Top/foo:Foo>w"} ]]
layer A, bind:

public module Top:
inst foo of Foo
inst bar of Foo
module Foo:
layerblock A:
wire w : UInt<8>
invalidate w
```
compiling with `firtool -parse-only test.fir` gives the following IR:
```mlir
module {
firrtl.circuit "Top" {
hw.hierpath private @nla [@Top::@sym, @Foo]
firrtl.layer @A bind {
}
firrtl.module @Top() attributes {convention = #firrtl} {
firrtl.instance foo sym @sym interesting_name @Foo()
firrtl.instance bar interesting_name @Foo()
}
firrtl.module private @Foo() {
firrtl.layerblock @A {
%w = firrtl.wire interesting_name {annotations = [{circt.nonlocal = @nla, class = "circt.test"}]} : !firrtl.uint<8>
%invalid_ui8 = firrtl.invalidvalue : !firrtl.uint<8>
firrtl.matchingconnect %w, %invalid_ui8 : !firrtl.uint<8>
}
}
}
}
```

After LowerLayers runs we get this IR:
```mlir
// -----// IR Dump After LowerLayers (firrtl-lower-layers) //----- //
firrtl.circuit "Top" {
sv.verbatim "`ifndef layers_Top_A\0A`define layers_Top_A" {output_file = #hw.output_file<"layers_Top_A.sv", excludeFromFileList>}
hw.hierpath private @nla [@Top::@sym, @Foo]
firrtl.module @Top() attributes {convention = #firrtl} {
firrtl.instance foo sym @sym @Foo()
firrtl.instance bar @Foo()
}
firrtl.module private @Foo_A() {
%w = firrtl.wire {annotations = [{circt.nonlocal = @nla, class = "circt.test"}]} : !firrtl.uint<8>
%c0_ui8 = firrtl.constant 0 : !firrtl.uint<8>
firrtl.matchingconnect %w, %c0_ui8 : !firrtl.uint<8>
}
firrtl.module private @Foo() {
firrtl.instance a {lowerToBind, output_file = #hw.output_file<"layers_Top_A.sv", excludeFromFileList>} @Foo_A()
}
sv.verbatim "`endif // layers_Top_A" {output_file = #hw.output_file<"layers_Top_A.sv", excludeFromFileList>}
}
```

This issue is that the non-local annotation on `wire w` is not updated when the wire is moved in to a module `Foo_A`, and corresponding hierpath operation still points to `@Foo`. I.e.
```hw.hierpath private @nla [@Top::@sym, @Foo]```
should be
```hw.hierpath private @nla [@Top::@sym, @Foo::@a_sym, @Foo_A]```
and the instance of `Foo_A` needs a new symbol `a_sym`.

Note that we can't just modify the original HierpathOp, since it may have other users which were not moved, so we must create a new Hierpath op.

This currently does not trigger a verifier error, which has a tracking issue here: https://github.com/llvm/circt/issues/4896
A similar issue was tackled here: https://github.com/llvm/circt/issues/6717

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.