Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lexical scope is not properly checked for ports of submodules #4405

Open
jackkoenig opened this issue Sep 18, 2024 · 2 comments
Open

Lexical scope is not properly checked for ports of submodules #4405

jackkoenig opened this issue Sep 18, 2024 · 2 comments
Labels

Comments

@jackkoenig
Copy link
Contributor

This is a good example of why we need to overhaul our lexical scope checking (usually called "visibility" within Chisel). We really ought to add a specific Scope datastructure that is used for all scopes (Module, when, and layer) rather than individual things for checking each one (and I think layer scopes aren't even properly checked at the moment!)

Type of issue: Bug Report

Please provide the steps to reproduce the problem:

Consider the following Scala-CLI using latest main:

//> 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._
// _root_ disambiguates from package chisel3.util.circt if user imports chisel3.util._
import _root_.circt.stage.ChiselStage

class Child extends Module {
  val in = IO(Input(UInt(8.W)))
  val out = IO(Output(UInt(8.W)))

  out := in
}

class Foo extends Module {
  val in = IO(Input(UInt(8.W)))
  val out = IO(Output(UInt(8.W)))

  var c: Child = null
  when (true.B) {
    c = Module(new Child)
  }
  c.in := in
  out := c.out
}

object Main extends App {
  println(
    ChiselStage.emitCHIRRTL(
      gen = new Foo
    )
  )
  println(
    ChiselStage.emitSystemVerilog(
      gen = new Foo,
      firtoolOpts = Array("-disable-all-randomization", "-strip-debug-info")
    )
  )
}

What is the current behavior?

This will print the buggy FIRRTL and show the firtool error:

FIRRTL version 4.0.0
circuit Foo :
  layer Verification, bind, "Verification" :
    layer Assert, bind, "Verification/Assert" :
    layer Assume, bind, "Verification/Assume" :
    layer Cover, bind, "Verification/Cover" :
  module Child : @[Users/koenig/work/t/scope/chisel-example.scala 11:7]
    input clock : Clock @[Users/koenig/work/t/scope/chisel-example.scala 11:7]
    input reset : Reset @[Users/koenig/work/t/scope/chisel-example.scala 11:7]
    input in : UInt<8> @[Users/koenig/work/t/scope/chisel-example.scala 12:14]
    output out : UInt<8> @[Users/koenig/work/t/scope/chisel-example.scala 13:15]

    connect out, in @[Users/koenig/work/t/scope/chisel-example.scala 15:7]

  public module Foo : @[Users/koenig/work/t/scope/chisel-example.scala 18:7]
    input clock : Clock @[Users/koenig/work/t/scope/chisel-example.scala 18:7]
    input reset : UInt<1> @[Users/koenig/work/t/scope/chisel-example.scala 18:7]
    input in : UInt<8> @[Users/koenig/work/t/scope/chisel-example.scala 19:14]
    output out : UInt<8> @[Users/koenig/work/t/scope/chisel-example.scala 20:15]

    when UInt<1>(0h1) : @[Users/koenig/work/t/scope/chisel-example.scala 23:17]
      inst Child of Child @[Users/koenig/work/t/scope/chisel-example.scala 24:15]
      connect Child.clock, clock
      connect Child.reset, reset
    connect Child.in, in @[Users/koenig/work/t/scope/chisel-example.scala 26:8]
    connect out, Child.out @[Users/koenig/work/t/scope/chisel-example.scala 27:7]
Exception in thread "main" circt.stage.phases.Exceptions$FirtoolNonZeroExitCode: /Users/koenig/Library/Caches/org.chipsalliance.llvm-firtool/1.86.0/bin/firtool returned a non-zero exit code. Note that this version of Chisel (7.0.0-M2+76-ecda00a5-SNAPSHOT) was published against firtool version 1.86.0.
------------------------------------------------------------------------------
ExitCode:
1
STDOUT:

STDERR:
<stdin>:25:13: error: use of unknown declaration 'Child'
    connect Child.in, in @[Users/koenig/work/t/scope/chisel-example.scala 26:8]
            ^

------------------------------------------------------------------------------

What is the expected behavior?

Chisel should error

What is the use case for changing the behavior?

@jackkoenig jackkoenig added the bug label Sep 18, 2024
@dtzSiFive
Copy link
Member

Yes, there's only scope checking for when's, and not layers (tested but also apparent from inspection re:WhenContext, so on).

FWIW I think the checking is wrong among sibling blocks for the same when as well:

//> 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._
// _root_ disambiguates from package chisel3.util.circt if user imports chisel3.util._
import _root_.circt.stage.ChiselStage

class Child extends Module {
  val in = IO(Input(UInt(8.W)))
  val out = IO(Output(UInt(8.W)))

  out := in
}

class Foo extends Module {
  val in = IO(Input(UInt(8.W)))
  val out = IO(Output(UInt(8.W)))

  var c: Bool = null
  when (true.B) {
    c = WireInit(Bool(), true.B)
    out := c
  } .otherwise {
    c := false.B
    out := c
  }
}

object Main extends App {
  println(
    ChiselStage.emitCHIRRTL(
      gen = new Foo
    )
  )
  println(
    ChiselStage.emitSystemVerilog(
      gen = new Foo,
      firtoolOpts = Array("-disable-all-randomization", "-strip-debug-info")
    )
  )
}

Which generates the following output:

circuit Foo :
  layer Verification, bind, "Verification" :
    layer Assert, bind, "Verification/Assert" :
    layer Assume, bind, "Verification/Assume" :
    layer Cover, bind, "Verification/Cover" :
  public module Foo : @[scope-when-else.scala 18:7]
    input clock : Clock @[scope-when-else.scala 18:7]
    input reset : UInt<1> @[scope-when-else.scala 18:7]
    input in : UInt<8> @[scope-when-else.scala 19:14]
    output out : UInt<8> @[scope-when-else.scala 20:15]

    when UInt<1>(0h1) : @[scope-when-else.scala 23:17]
      wire _WIRE : UInt<1> @[scope-when-else.scala 24:17]
      connect _WIRE, UInt<1>(0h1) @[scope-when-else.scala 24:17]
      connect out, _WIRE @[scope-when-else.scala 25:9]
    else :
      connect _WIRE, UInt<1>(0h0) @[scope-when-else.scala 27:7]
      connect out, _WIRE @[scope-when-else.scala 28:9]



Exception in thread "main" circt.stage.phases.Exceptions$FirtoolNonZeroExitCode: /home/will/.nix-profile/bin/firtool returned a non-zero exit code. Note that this version of Chisel (7.0.0-M2+76-ecda00a5-SNAPSHOT) was published against firtool version 1.86.0.
------------------------------------------------------------------------------
ExitCode:
1
STDOUT:

STDERR:
<stdin>:18:15: error: use of unknown declaration '_WIRE'
      connect _WIRE, UInt<1>(0h0) @[scope-when-else.scala 27:7]
              ^

------------------------------------------------------------------------------

As you say, probably motivates a scope overhaul 👍 .
I can file separate issue if you'd prefer? But anyway just wanted to drop this somewhere.

@jackkoenig
Copy link
Contributor Author

Thanks for the extra example! I think it's fine as part of this issue but feel free to file a separate one if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants