Skip to content

Commit

Permalink
Fix ActualDirection calculation from SpecifiedDirection
Browse files Browse the repository at this point in the history
Unspecified direction maps to Output while Flip maps to Input.
Previously, ActualDirection.fromSpecified would return Unspecified for
either Unspecified or Flipped input. This in turn resulted in Bundles
mixing Unspecified and Outputs as being "bidirectional" despite the fact
that they are actually unidirectional (or passive).

This change makes the direction calculation more consistent albeit at
the cost of marking the directions of unspecified Data as Output.
  • Loading branch information
jackkoenig committed Jun 21, 2024
1 parent 00c7a62 commit 4f22aa2
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 9 deletions.
7 changes: 1 addition & 6 deletions core/src/main/scala/chisel3/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1088,12 +1088,7 @@ abstract class Record extends Aggregate {
direction = ActualDirection.fromChildren(childDirections, resolvedDirection) match {
case Some(dir) => dir
case None =>
val resolvedDirection = SpecifiedDirection.fromParent(parentDirection, specifiedDirection)
resolvedDirection match {
case SpecifiedDirection.Unspecified => ActualDirection.Bidirectional(ActualDirection.Default)
case SpecifiedDirection.Flip => ActualDirection.Bidirectional(ActualDirection.Flipped)
case _ => ActualDirection.Bidirectional(ActualDirection.Default)
}
throwException(s"Internal Error! Unhandled directionality of children: $childDirections for $this!")
}
setElementRefs()

Expand Down
9 changes: 6 additions & 3 deletions core/src/main/scala/chisel3/Data.scala
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,13 @@ object ActualDirection {

case class Bidirectional(dir: BidirectionalDirection) extends ActualDirection

/** Converts a `SpecifiedDirection` to an `ActualDirection`
*
* Implements the Chisel convention that Flip is Input and unspecified is Output.
*/
def fromSpecified(direction: SpecifiedDirection): ActualDirection = direction match {
case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => ActualDirection.Unspecified
case SpecifiedDirection.Output => ActualDirection.Output
case SpecifiedDirection.Input => ActualDirection.Input
case SpecifiedDirection.Output | SpecifiedDirection.Unspecified => ActualDirection.Output
case SpecifiedDirection.Input | SpecifiedDirection.Flip => ActualDirection.Input
}

/** Determine the actual binding of a container given directions of its children.
Expand Down
40 changes: 40 additions & 0 deletions src/test/scala/chiselTests/Direction.scala
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,45 @@ class DirectionSpec extends ChiselPropSpec with Matchers with Utils {
assert(emitted.contains("connect io.monitor.valid, io.driver.valid"))
assert(emitted.contains("connect io.monitor.ready, io.driver.ready"))
}

property("Output mixed with unspecified directions should report Output") {
class MyBundle extends Bundle {
val foo = UInt(8.W)
val bar = Output(UInt(8.W))
}
class MyModule extends RawModule {
val w = Wire(new MyBundle)
assert(DataMirror.specifiedDirectionOf(w) == SpecifiedDirection.Unspecified)
assert(DataMirror.specifiedDirectionOf(w.foo) == SpecifiedDirection.Unspecified)
assert(DataMirror.specifiedDirectionOf(w.bar) == SpecifiedDirection.Output)
assert(DataMirror.directionOf(w) == Direction.Output)
assert(DataMirror.directionOf(w.foo) == Direction.Output)
assert(DataMirror.directionOf(w.bar) == Direction.Output)

}
val chirrtl = ChiselStage.emitCHIRRTL(new MyModule)
assert(chirrtl.contains("wire w : { foo : UInt<8>, bar : UInt<8>}"))
}

property("Input mixed with Flipped directions should report Input") {
class MyBundle extends Bundle {
val foo = Flipped(UInt(8.W))
val bar = Input(UInt(8.W))
}
class MyModule extends RawModule {
val w = Wire(new MyBundle)
assert(DataMirror.specifiedDirectionOf(w) == SpecifiedDirection.Unspecified)
assert(DataMirror.specifiedDirectionOf(w.foo) == SpecifiedDirection.Flip)
assert(DataMirror.specifiedDirectionOf(w.bar) == SpecifiedDirection.Input)
assert(DataMirror.directionOf(w) == Direction.Input)
assert(DataMirror.directionOf(w.foo) == Direction.Input)
assert(DataMirror.directionOf(w.bar) == Direction.Input)

}
val chirrtl = ChiselStage.emitCHIRRTL(new MyModule)
assert(chirrtl.contains("wire w : { flip foo : UInt<8>, flip bar : UInt<8>}"))
}

property("Bugfix: marking Vec fields with mixed directionality as Output/Input clears inner directions") {
class Decoupled extends Bundle {
val bits = UInt(3.W)
Expand Down Expand Up @@ -559,4 +598,5 @@ class DirectionSpec extends ChiselPropSpec with Matchers with Utils {
val emitted: String = ChiselStage.emitCHIRRTL(new MyModule)
assert(emitted.contains("Probe<const UInt<1>>"))
}

}

0 comments on commit 4f22aa2

Please sign in to comment.