chipsalliance / chipsalliance/chisel

Double-boring can lead to FIRRTL with sink on the RHS of a propassign

Open
#5,038 0 comments 0 reactions 0 assignees View on GitHub
bug
Dominant language
Scala
Stars
4.8k
Forks
658
Avg merge
18h 59m
Merged PRs (30d)
14

Description

Ran into the following issue in the wild. If you use `BoringUtils.bore` on a value that is already the result of a `BoringUtils.bore`, Chisel can generate `propassign`s in the final FIRRTL that have an automatically-bored output port on the right-hand side.

Consider this example:
```scala
//> using repository https://central.sonatype.com/repository/maven-snapshots
//> using scala 2.13.16
//> using dep org.chipsalliance::chisel:7.1.1
//> using plugin org.chipsalliance:::chisel-plugin:7.1.1
//> using options -unchecked -deprecation -language:reflectiveCalls -feature -Xcheckinit
//> using options -Xfatal-warnings -Ywarn-dead-code -Ywarn-unused -Ymacro-annotations

import chisel3._
import chisel3.properties._
import chisel3.util.experimental.BoringUtils
// _root_ disambiguates from package chisel3.util.circt if user imports chisel3.util._
import _root_.circt.stage.ChiselStage

class Foo extends RawModule {
val bar = Module(new Bar)
val a = BoringUtils.bore(bar.w)
val b = BoringUtils.bore(a)
}

class Bar extends RawModule {
val w = WireInit(Property(9001))
}

object Main extends App {
println(ChiselStage.emitCHIRRTL(gen = new Foo))
println(ChiselStage.emitHWDialect(gen = new Foo))
}
```
Running this through `scala-cli` produces the following problematic snippet of CHIRRTL/FIRRTL:
```firrtl
FIRRTL version 6.0.0
circuit Foo :
[...]
module Bar :
output a_bore : Integer
output b_bore : Integer

wire w : Integer
propassign w, Integer(9001)
propassign a_bore, w
propassign b_bore, a_bore ; <--- second bore tries to reuse first bore

public module Foo :
inst bar of Bar
```
Which then fails in firtool's flow checking for connects and propassigns:
```
error: connect has invalid flow: the source expression "a_bore" has sink flow, expected source or duplex flow
```
We try to make `BoringUtils.bore` behave like a no-op if the targeted value is already available in the current module, and just pass that through. We also make an effort to reuse existing output ports of a module during boring. This feels like a corner case where the previously-bored output `a_bore` is tried to be reused, but instead of following the output upwards to the instance output, which would be a source, the code tries to bore a second output port `b_bore` but then reuse the existing `a_bore` as its value, which is a sink and thus can't be on the RHS of a propassign.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.