Skip to content

Commit

Permalink
Fixup and unit tests for D/I of IOs without explicit Input/Output (#2792
Browse files Browse the repository at this point in the history
)
  • Loading branch information
mwachs5 authored Nov 8, 2022
1 parent fce8394 commit f24a624
Show file tree
Hide file tree
Showing 2 changed files with 322 additions and 28 deletions.
66 changes: 38 additions & 28 deletions core/src/main/scala/chisel3/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,38 @@ object Module extends SourceInfoDoc {

module
}

/** Assign directionality on any IOs that are still Unspecified/Flipped
*
* Chisel2 did not require explicit direction on nodes
* (unspecified treated as output, and flip on nothing was input).
* As of 3.6, chisel3 is now also using these semantics, so we need to make it work
* even for chisel3 code.
* This assigns the explicit directions required by both semantics on all Bundles.
* This recursively walks the tree, and assigns directions if no explicit
* direction given by upper-levels (override Input / Output)
*/
private[chisel3] def assignCompatDir(data: Data): Unit = {
data match {
case data: Element => data._assignCompatibilityExplicitDirection
case data: Aggregate =>
data.specifiedDirection match {
// Recurse into children to ensure explicit direction set somewhere
case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip =>
data match {
case record: Record =>
record.elementsIterator.foreach(assignCompatDir(_))
case vec: Vec[_] =>
vec.elementsIterator.foreach(assignCompatDir(_))
assignCompatDir(vec.sample_element) // This is used in fromChildren computation
}
case SpecifiedDirection.Input | SpecifiedDirection.Output =>
// forced assign, nothing to do
// The .bind algorithm will automatically assign the direction here.
// Thus, no implicit assignment is necessary.
}
}
}
}

/** Abstract base class for Modules, which behave much like Verilog modules.
Expand Down Expand Up @@ -252,6 +284,10 @@ package internal {
new ClonePorts(proto.getChiselPorts :+ ("io" -> b._io.get): _*)
case _ => new ClonePorts(proto.getChiselPorts: _*)
}
// getChiselPorts (nor cloneTypeFull in general)
// does not recursively copy the right specifiedDirection,
// still need to fix it up here.
Module.assignCompatDir(clonePorts)
clonePorts.bind(PortBinding(cloneParent))
clonePorts.setAllParents(Some(cloneParent))
cloneParent._portsRecord = clonePorts
Expand Down Expand Up @@ -524,35 +560,9 @@ package experimental {
* io, then do operations on it. This binds a Chisel type in-place (mutably) as an IO.
*/
protected def _bindIoInPlace(iodef: Data)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): Unit = {
// Compatibility code: Chisel2 did not require explicit direction on nodes
// (unspecified treated as output, and flip on nothing was input).
// However, we are going to go back to Chisel2 semantics, so we need to make it work
// even for chisel3 code.
// This assigns the explicit directions required by both semantics on all Bundles.
// This recursively walks the tree, and assigns directions if no explicit
// direction given by upper-levels (override Input / Output)
def assignCompatDir(data: Data): Unit = {
data match {
case data: Element => data._assignCompatibilityExplicitDirection
case data: Aggregate =>
data.specifiedDirection match {
// Recurse into children to ensure explicit direction set somewhere
case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip =>
data match {
case record: Record =>
record.elementsIterator.foreach(assignCompatDir(_))
case vec: Vec[_] =>
vec.elementsIterator.foreach(assignCompatDir(_))
}
case SpecifiedDirection.Input | SpecifiedDirection.Output =>
// forced assign, nothing to do
// Note this is because Input and Output recurse down their types to align all fields to that SpecifiedDirection
// Thus, no implicit assigment is necessary.
}
}
}

assignCompatDir(iodef)
// Assign any signals (Chisel or chisel3) with Unspecified/Flipped directions to Output/Input
Module.assignCompatDir(iodef)

iodef.bind(PortBinding(this))
_ports += iodef -> sourceInfo
Expand Down
284 changes: 284 additions & 0 deletions src/test/scala/chiselTests/CompatibilityInteroperabilitySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,290 @@ class CompatibilityInteroperabilitySpec extends ChiselFlatSpec {
compile(new Top(false))
}

"A Chisel.Bundle with only unspecified directions" should "work with D/I" in {

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
}
}
compile(new Chisel3.Example)
}

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

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
}
}
compile(new Chisel3.Example)
}

"A Chisel.Bundle with only unspecified vec direction" should "work with D/I" in {

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
}
}
compile(new Chisel3.Example)
}

"A chisel3.Bundle with only unspecified directions" should "work with D/I" in {

// This test is NOT expected to work on 3.5.x, it should throw an error in the IO construction.

object Chisel3 {
import chisel3._
import chisel3.experimental.hierarchy.{instantiable, public, Instance}

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

@instantiable
class MyModule extends Module {
@public val io = IO(new BiDirUnspecifiedBundle)
io <> DontCare
}

class Example extends Module {
val mod = Module(new MyModule())
mod.io.in <> DontCare
val inst = Instance(mod.toDefinition)
inst.io.in <> mod.io.out
mod.io.in <> inst.io.out
}
}
compile(new Chisel3.Example)
}

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

// This test is NOT expected to work on 3.5.x, it should throw an error in the IO construction.

object Chisel3 {
import chisel3._
import chisel3.experimental.hierarchy.{instantiable, public, Instance}

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

@instantiable
class MyModule extends Module {
@public val io = IO(new BiDirMixedBundle)
io <> DontCare
}
class Example extends Module {
val mod = Module(new MyModule)
mod.io.in <> DontCare
val inst = Instance(mod.toDefinition)
inst.io.in <> mod.io.out
mod.io.in <> inst.io.out
}
}
compile(new Chisel3.Example)
}

"A chisel3.Bundle with only unspecified vec direction" should "work with D/I" in {

// This test is NOT expected to work on 3.5.x, it should throw an error in the IO construction

object Chisel3 {
import chisel3._
import chisel3.experimental.hierarchy.{instantiable, public, Instance}

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

@instantiable
class MyModule extends Module {
@public val io = IO(new BiDirUnspecifiedVecBundle)
io <> DontCare
}

class Example extends Module {
val mod = Module(new MyModule())
mod.io.in <> DontCare
val inst = Instance(mod.toDefinition)
inst.io.in <> mod.io.out
mod.io.in <> inst.io.out
}
}
compile(new Chisel3.Example)
}

"A chisel3.Bundle with only unspecified vec direction within an unspecified direction parent Bundle" should "work with D/I" in {

// This test is NOT expected to work in 3.5.x, it should throw an error in the IO construction.

object Chisel3 {
import chisel3._
import chisel3.experimental.hierarchy.{instantiable, public, Instance}

class UnspecifiedVecBundle extends Bundle {
val vec = Vec(3, Bool())
}

class UnspecifiedParentBundle extends Bundle {
val out = new UnspecifiedVecBundle
}

@instantiable
class MyModule extends Module {
@public val io = IO(new UnspecifiedParentBundle)
io <> DontCare
}

class Example extends Module {
val mod = Module(new MyModule())

val wire = Wire(new UnspecifiedParentBundle)
wire.out.vec <> mod.io.out.vec
val inst = Instance(mod.toDefinition)
wire.out.vec <> inst.io.out.vec

}
}
compile(new Chisel3.Example)
}

"A undirectioned Chisel.Bundle used in a MixedVec " should "bulk connect in import chisel3._ code correctly" in {

// This test is NOT expected to work on 3.5.x, it should throw an error in the IO construction.

object Compat {

import Chisel._
import chisel3.util.MixedVec

class ChiselModule extends Module {
val io = IO(new Bundle {
val out = MixedVec(Seq.fill(3) { Bool() })
val in = Flipped(MixedVec(Seq.fill(3) { Bool() }))
})
io.out := RegNext(io.in)
}

}
object Chisel3 {
import chisel3._

class Chisel3Module extends Compat.ChiselModule

class Example extends Module {
val oldMod = Module(new Compat.ChiselModule)
val newMod = Module(new Chisel3Module)

oldMod.io.in <> DontCare
newMod.io.in <> DontCare

}
}
compile(new Chisel3.Example)
}

"A undirectioned Chisel.Bundle with Records with undirectioned and directioned fields " should "work" in {

// This test should fail on 3.5.x

object Compat {

import Chisel._

class ChiselModule(gen: () => Data) extends Module {
val io = IO(new Bundle {
val mixed = new Chisel3.MyRecord(gen)
})
}

}
object Chisel3 {
import chisel3._
import scala.collection.immutable.SeqMap

class MyRecord(gen: () => Data) extends Record with chisel3.experimental.AutoCloneType {
val elements = SeqMap("genDirectioned" -> Output(gen()), "genUndirectioned" -> gen())
}

class Example extends Module {
val newMod = Module(new Compat.ChiselModule(() => Bool()))
}
}
compile(new Chisel3.Example)
}

"A BlackBox with Chisel._ fields in its IO" should "bulk connect in import chisel3._ code correctly" in {
object Compat {
import Chisel._
Expand Down

0 comments on commit f24a624

Please sign in to comment.