chipsalliance / chisel

Chisel: A Modern Hardware Design Language
https://www.chisel-lang.org/
Apache License 2.0
3.93k stars 589 forks source link

skipIfAlreadyInLayerBlock is preserved into instances #4404

Closed seldridge closed 1 week ago

seldridge commented 1 week ago

The layer stack should be cleared when entering a new module. Currently, it is not, and this will create situations where layer blocks in submodules may get surprisingly dropped.

//> using repository "sonatype-s01:snapshots"
//> using scala "2.13.14"
//> using dep "org.chipsalliance::chisel:7.0.0-M2+76-ecda00a5-SNAPSHOT"
//> using plugin "org.chipsalliance:::chisel-plugin:7.0.0-M2+76-ecda00a5-SNAPSHOT"
//> using options "-unchecked", "-deprecation", "-language:reflectiveCalls", "-feature", "-Xcheckinit", "-Xfatal-warnings", "-Ywarn-dead-code", "-Ywarn-unused", "-Ymacro-annotations"

import chisel3._
import chisel3.layer.{Layer, Convention}
import circt.stage.ChiselStage

object LayerA extends Layer(Convention.Bind)

class Bar extends RawModule {
  layer.block(LayerA, skipIfAlreadyInBlock=true) {
    val a = Wire(Bool())
  }
}

class Foo extends RawModule {
  layer.block(LayerA) {
    val bar = Module(new Bar)
  }
}

object Main extends App {
  println(ChiselStage.emitCHIRRTL(new Foo))
}

This produces the following:

FIRRTL version 4.0.0
circuit Foo :
  layer LayerA, bind, "LayerA" :
  layer Verification, bind, "Verification" :
    layer Assert, bind, "Verification/Assert" :
    layer Assume, bind, "Verification/Assume" :
    layer Cover, bind, "Verification/Cover" :
  module Bar :

    wire a : UInt<1>

  public module Foo :

    layerblock LayerA :
      inst bar of Bar
dtzSiFive commented 1 week ago

layerStack not being cleared across modules causes multiple other issues (anything using the stack from a module instantiated under a layerblock), maybe rename this (or I can file issues if you'd prefer) to reflect that?

For example:

//> using repository "sonatype-s01:snapshots"
//> using scala "2.13.14"
//> using dep "org.chipsalliance::chisel:7.0.0-M2+76-ecda00a5-SNAPSHOT"
//> using plugin "org.chipsalliance:::chisel-plugin:7.0.0-M2+76-ecda00a5-SNAPSHOT"
//> using options "-unchecked", "-deprecation", "-language:reflectiveCalls", "-feature", "-Xcheckinit", "-Xfatal-warnings", "-Ywarn-dead-code", "-Ywarn-unused", "-Ymacro-annotations"

import chisel3._
import chisel3.layer.{Layer, LayerConfig}
import circt.stage.ChiselStage
import chisel3.probe.{define, read, Probe, ProbeValue}

object A extends layer.Layer(layer.LayerConfig.Extract()) {
  object B extends layer.Layer(layer.LayerConfig.Extract())
}

class Bar extends RawModule {
  layer.block(A.B) {
    val w = WireInit(Bool(), true.B)
  }
}

class Foo extends RawModule {
  layer.block(A) {
     val bar = Module(new Bar())
  }
}

object Main extends App {
  println(ChiselStage.emitCHIRRTL(new Foo))
  println(ChiselStage.emitSystemVerilog(new Foo))
}

produces:

FIRRTL version 4.0.0
circuit Foo :
  layer A, bind, "A" : @[layerstack-nest.scala 12:24]
    layer B, bind, "A/B" : @[layerstack-nest.scala 13:26]
  layer Verification, bind, "Verification" :
    layer Assert, bind, "Verification/Assert" :
    layer Assume, bind, "Verification/Assume" :
    layer Cover, bind, "Verification/Cover" :
  module Bar : @[layerstack-nest.scala 16:7]

    layerblock B : @[layerstack-nest.scala 17:20]
      wire w : UInt<1> @[layerstack-nest.scala 18:21]
      connect w, UInt<1>(0h1) @[layerstack-nest.scala 18:21]

  public module Foo : @[layerstack-nest.scala 22:7]

    layerblock A : @[layerstack-nest.scala 23:18]
      inst bar of Bar @[layerstack-nest.scala 24:22]

Where layerblock B is illegal but the wrapping layerblock A is omitted due to A already being on the stack. Note that the skip option is not used here.