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

Definition/Instance API does not assign unspecified directions on ports #2794

Closed
mwachs5 opened this issue Oct 20, 2022 · 1 comment
Closed

Comments

@mwachs5
Copy link
Contributor

mwachs5 commented Oct 20, 2022

Type of issue: Bug Report

Please provide the steps to reproduce the problem:

Some unit tests that demonstrate the problem are here: https://github.com/chipsalliance/chisel3/blob/60d32653304904a03d432735e30204cd1730aa26/src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala#L394, copied below

"A Chisel.Bundle with only unspecified directions" should "work with D/I" in {
    /* Fails on the instance <>:

    [info] A Chisel.Bundle with only unspecified directions
    [info] - should work with D/I *** FAILED ***
    [info]   java.lang.RuntimeException: Unexpected port element direction 'Unspecified'
    [info]   at chisel3.internal.BindingDirection$.from(Binding.scala:62)
    [info]   at chisel3.internal.BiConnect$.elemConnect(BiConnect.scala:357)
    [info]   at chisel3.internal.BiConnect$.connect(BiConnect.scala:103)
     */

    object Compat {
      import Chisel._
      import chisel3.experimental.hierarchy.{instantiable, public}

      class CompatBiDirUnspecifiedBundle extends Bundle {
        val out = Bool()
        val in = Flipped(Bool())
      }

      @instantiable
      class CompatModule extends Module {
        @public val io = IO(new CompatBiDirUnspecifiedBundle)
      }
    }

    object Chisel3 {
      import chisel3._
      import chisel3.experimental.hierarchy.Instance
      class Example extends Module {
        val mod = Module(new Compat.CompatModule())
        mod.io.in <> DontCare
        val inst = Instance(mod.toDefinition)
        inst.io.in <> mod.io.out
        mod.io.in <> inst.io.out
      }
    }
    (new chisel3.stage.ChiselStage).emitVerilog(new Chisel3.Example, Array("--full-stacktrace"))
  }

  "A Chisel.Bundle with mixed Specified and Unspecified directions" should "work with D/I" in {

    /**
      *       A Chisel.Bundle with mixed Specified and Unspecified directions
      *       [info] - should work with D/I *** FAILED ***
      *       [info]   java.lang.RuntimeException: Unexpected port element direction 'Unspecified'
      *       [info]   at chisel3.internal.BindingDirection$.from(Binding.scala:62)
      *       [info]   at chisel3.internal.BiConnect$.elemConnect(BiConnect.scala:357)
      *       [info]   at chisel3.internal.BiConnect$.connect(BiConnect.scala:103)
      *      [info]   at chisel3.Data.bulkConnect(Data.scala:658)
      */

    object Compat {
      import Chisel._
      import chisel3.experimental.hierarchy.{instantiable, public}

      class CompatBiDirMixedBundle extends Bundle {
        val out = Bool()
        val in = Flipped(Bool())
        val explicit = Output(Bool())
      }

      @instantiable
      class CompatModule extends Module {
        @public val io = IO(new CompatBiDirMixedBundle)
      }
    }

    object Chisel3 {
      import chisel3._
      import chisel3.experimental.hierarchy.Instance
      class Example extends Module {
        val mod = Module(new Compat.CompatModule)
        mod.io.in <> DontCare
        val inst = Instance(mod.toDefinition)
        inst.io.in <> mod.io.out
        mod.io.in <> inst.io.out
      }
    }
    (new chisel3.stage.ChiselStage).emitVerilog(new Chisel3.Example, Array("--full-stacktrace"))
  }
  "A Chisel.Bundle with only unspecified vec direction" should "work with D/I" in {

    /**
      * A Chisel.Bundle with only unspecified vec direction
      * [info] - should work with D/I *** FAILED ***
      * [info]   java.lang.RuntimeException: Unexpected port element direction 'Unspecified'
      * [info]   at chisel3.internal.BindingDirection$.from(Binding.scala:62)
      * [info]   at chisel3.internal.BiConnect$.elemConnect(BiConnect.scala:357)
      * [info]   at chisel3.internal.BiConnect$.connect(BiConnect.scala:103)
      * [info]   at chisel3.internal.BiConnect$.$anonfun$connect$1(BiConnect.scala:129)
      * [info]   at scala.collection.immutable.Range.foreach$mVc$sp(Range.scala:158)
      * [info]   at chisel3.internal.BiConnect$.connect(BiConnect.scala:126)
      * [info]   at chisel3.Data.bulkConnect(Data.scala:658
      */
    object Compat {
      import Chisel._
      import chisel3.experimental.hierarchy.{instantiable, public}

      class CompatBiDirUnspecifiedVecBundle extends Bundle {
        val out = Vec(3, Bool())
        val in = Flipped(Vec(3, Bool()))
      }

      @instantiable
      class CompatModule extends Module {
        @public val io = IO(new CompatBiDirUnspecifiedVecBundle)
      }
    }

    object Chisel3 {
      import chisel3._
      import chisel3.experimental.hierarchy.Instance
      class Example extends Module {
        val mod = Module(new Compat.CompatModule())
        mod.io.in <> DontCare
        val inst = Instance(mod.toDefinition)
        inst.io.in <> mod.io.out
        mod.io.in <> inst.io.out
      }
    }
    (new chisel3.stage.ChiselStage).emitVerilog(new Chisel3.Example, Array("--full-stacktrace"))
  }

What is the current behavior?

An Instance with Aggregates that have unspecified directions is legal in normal Module code, but when we make an Instance of it the unspecified directions are not assigned (this problem is actually worse on master than on 3.5.x because we now have it legal for all chisel3 Bundles to have unspecified directions, and are relying on fixup code that does not run for Instances to give them their specified directions). This results in the Instance being successfully created (the .bind of the ports to the parent without specified directions works for some reason) but then crashing when we try to connect to it as the directon is not actually specified.

What is the expected behavior?

I should be able to connect to IOs on instances just as if it were a Module, and not get crashes about Unspecified directions.

Please tell us about your environment:

Issue exists but with different failure modes on master and 3.5.x

Other Information

What is the use case for changing the behavior?

Making the D/I match the expectations of Module

@mwachs5 mwachs5 changed the title Definition/Instance API does not assign compatibility directions Definition/Instance API does not assign unspecified directions on ports Oct 20, 2022
mergify bot added a commit that referenced this issue Nov 10, 2022
…2792) (#2834)

* Fixup and unit tests for D/I of IOs without explicit Input/Output (#2792)

(cherry picked from commit f24a624)

# Conflicts:
#	core/src/main/scala/chisel3/Module.scala

* Resolve backport conflicts

Co-authored-by: Megan Wachs <megan@sifive.com>
Co-authored-by: Jack Koenig <koenig@sifive.com>
@mwachs5
Copy link
Contributor Author

mwachs5 commented Apr 4, 2023

closed by #2792

@mwachs5 mwachs5 closed this as completed Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant