llvm / llvm/circt

[FIRRTL] DontTouchAnnotation as Symbol and Deduplication

Open
#2,531 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

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

Description

The modeling of `DontTouchAnnotation` as a symbol destroys some information that may be necessary for an SFC-exact implementation of deduplication.

Consider the following circuit. Modules `A` and `A_` are structurally equivalent and should deduplicate. `A` has a `DontTouchAnnotation` on a wire inside it. This is not supposed to block deduplication.

```scala
circuit Top : %[[
{
"class":"firrtl.transforms.DontTouchAnnotation",
"target":"Top.A.b"
}
]]
module Top :
inst a1 of A
inst a2 of A_
module A :
output x: UInt<1>
wire b: UInt<1>
b is invalid
x <= b
module A_ :
output x: UInt<1>
wire b: UInt<1>
b is invalid
x <= b
```

When we parse this in, this specific annotations becomes a symbol `@b`:

```mlir
module {
firrtl.circuit "Top" {
firrtl.module @Top() {
%a1_x = firrtl.instance a1 @A(out x: !firrtl.uint<1>)
%a2_x = firrtl.instance a2 @A_(out x: !firrtl.uint<1>)
}
firrtl.module @A(out %x: !firrtl.uint<1>) {
%b = firrtl.wire sym @b : !firrtl.uint<1>
%invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1>
firrtl.connect %b, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1>
firrtl.connect %x, %b : !firrtl.uint<1>, !firrtl.uint<1>
}
firrtl.module @A_(out %x: !firrtl.uint<1>) {
%b = firrtl.wire : !firrtl.uint<1>
%invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1>
firrtl.connect %b, %invalid_ui1 : !firrtl.uint<1>, !firrtl.uint<1>
firrtl.connect %x, %b : !firrtl.uint<1>, !firrtl.uint<1>
}
}
}
```

After deduplication, the circuit looks like with `@b` applied to the deduplicated wire. The effect of the annotation now applies to both wires:
```mlir
module {
firrtl.circuit "Top" {
firrtl.module @Top() {
%a1_x = firrtl.instance a1 @A(out x: !firrtl.uint<1>)
%a2_x = firrtl.instance a2 @A(out x: !firrtl.uint<1>)
}
firrtl.module @A(out %x: !firrtl.uint<1>) {
%invalid_ui1 = firrtl.invalidvalue : !firrtl.uint<1>
%b = firrtl.wire sym @b : !firrtl.uint<1>
firrtl.connect %b, %invalid_ui1 {annotations = []} : !firrtl.uint<1>, !firrtl.uint<1>
firrtl.connect %x, %b {annotations = []} : !firrtl.uint<1>, !firrtl.uint<1>
}
}
}
```

In contrast, the SFC produces a non-local annotation targeting the original instance's wire:
```scala
circuit Top :
module A :
output x : UInt<1>

wire b : UInt<1> @[dont-touch.scala 33:15]
b is invalid @[dont-touch.scala 34:5]
x <= b @[dont-touch.scala 36:5]

module Top :

inst a1 of A @[dont-touch.scala 47:18]
inst a2 of A @[dont-touch.scala 48:18]
```
```json
[
{
"class":"firrtl.transforms.DontTouchAnnotation",
"target":"~Top|Top/a1:A>b"
}
]
```

I don't have an opinion or evidence that this is problematic, yet. This is just reporting an SFC deviation.

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.

Research direction

No source file or test is named, and the report does not specify a desired change. Start by tracing FIRRTL DontTouchAnnotation handling through deduplication and comparing the result with the SFC output shown. Done requires establishing whether symbolizing @b changes annotation scope and defining the expected behavior.

Written by the indexing model from the issue text.

Assessment

Domain
compilers
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.