Skip to content

Commit

Permalink
Remove val io
Browse files Browse the repository at this point in the history
Chisel projects no longer need -Xsource:2.11 when compiling with Scala
2.12.

Autowrapping of "val io" for compatibility mode Modules is now
implemented using reflection instead of calling the virtual method.

Also move Chisel.BlackBox to new chisel3.internal.LegacyBlackBox
  • Loading branch information
jackkoenig committed Jan 21, 2021
1 parent b88ae1f commit 8a73362
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 57 deletions.
4 changes: 0 additions & 4 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,6 @@ lazy val commonSettings = Seq (
scalaVersion := "2.12.12",
crossScalaVersions := Seq("2.12.12"),
scalacOptions := Seq("-deprecation", "-feature",
// We're building with Scala > 2.11, enable the compile option
// switch to support our anonymous Bundle definitions:
// https://github.com/scala/bug/issues/10047
"-Xsource:2.11"
),
libraryDependencies += "org.scala-lang" % "scala-reflect" % scalaVersion.value,
addCompilerPlugin("org.scalamacros" % "paradise" % "2.1.1" cross CrossVersion.full),
Expand Down
3 changes: 1 addition & 2 deletions build.sc
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ trait CommonModule extends CrossSbtModule with PublishModule {
override def scalacOptions = T {
super.scalacOptions() ++ Agg(
"-deprecation",
"-feature",
"-Xsource:2.11"
"-feature"
)
}

Expand Down
18 changes: 7 additions & 11 deletions core/src/main/scala/chisel3/BlackBox.scala
Original file line number Diff line number Diff line change
Expand Up @@ -134,24 +134,20 @@ package experimental {
*/
abstract class BlackBox(val params: Map[String, Param] = Map.empty[String, Param])(implicit compileOptions: CompileOptions) extends BaseBlackBox {

@deprecated("Removed for causing issues in Scala 2.12+. You remain free to define io Bundles " +
"in your BlackBoxes, but you cannot rely on an io field in every BlackBox. " +
"For more information, see: https://github.com/freechipsproject/chisel3/pull/1550.",
"Chisel 3.4"
)
def io: Record

// Private accessor to reduce number of deprecation warnings
private[chisel3] def _io: Record = io
// Find a Record port named "io" for purposes of stripping the prefix
private[chisel3] lazy val _io: Record =
this.findPort("io")
.collect { case r: Record => r } // Must be a Record
.getOrElse(null) // null handling occurs in generateComponent

// Allow access to bindings from the compatibility package
protected def _compatIoPortBound() = portsContains(_io)

private[chisel3] override def generateComponent(): Component = {
_compatAutoWrapPorts() // pre-IO(...) compatibility hack

// Restrict IO to just io, clock, and reset
require(_io != null, "BlackBox must have io")
// Restrict IO to just io, clock, and reset
require(_io != null, "BlackBox must have a port named 'io' of type Record!")
require(portsContains(_io), "BlackBox must have io wrapped in IO(...)")
require(portsSize == 1, "BlackBox must only have io as IO")

Expand Down
5 changes: 5 additions & 0 deletions core/src/main/scala/chisel3/Module.scala
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ package experimental {
// mainly for compatibility purposes.
protected def portsContains(elem: Data): Boolean = _ports contains elem

// This is dangerous because it can be called before the module is closed and thus there could
// be more ports and names have not yet been finalized.
// This should only to be used during the process of closing when it is safe to do so.
private[chisel3] def findPort(name: String): Option[Data] = _ports.find(_.seedOpt.contains(name))

protected def portsSize: Int = _ports.size

/** Generates the FIRRTL Component (Module or Blackbox) of this Module.
Expand Down
99 changes: 75 additions & 24 deletions core/src/main/scala/chisel3/RawModule.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package chisel3

import scala.collection.mutable.{ArrayBuffer, HashMap}
import scala.util.Try
import scala.collection.JavaConversions._
import scala.language.experimental.macros

Expand Down Expand Up @@ -150,40 +151,62 @@ trait RequireSyncReset extends Module {
override private[chisel3] def mkReset: Bool = Bool()
}

package internal {
package object internal {

// Private reflective version of "val io" to maintain Chisel.Module semantics without having
// io as a virtual method. See https://github.com/freechipsproject/chisel3/pull/1550 for more
// information about the removal of "val io"
private def reflectivelyFindValIO(self: BaseModule): Record = {
// Java reflection is faster and works for the common case
def tryJavaReflect: Option[Record] = Try {
self.getClass.getMethod("io").invoke(self).asInstanceOf[Record]
}.toOption
// Anonymous subclasses don't work with Java reflection, so try slower, Scala reflection
def tryScalaReflect: Option[Record] = {
val ru = scala.reflect.runtime.universe
import ru.{Try => _, _}
val m = ru.runtimeMirror(self.getClass.getClassLoader)
val im = m.reflect(self)
val tpe = im.symbol.toType
// For some reason, in anonymous subclasses, looking up the Term by name (TermName("io"))
// hits an internal exception. Searching for the term seems to work though so we use that.
val ioTerm: Option[TermSymbol] = tpe.decls.collectFirst {
case d if d.name.toString == "io" && d.isTerm => d.asTerm
}
ioTerm.flatMap { term =>
Try {
im.reflectField(term).get.asInstanceOf[Record]
}.toOption
}
}

tryJavaReflect
.orElse(tryScalaReflect)
.map(_.autoSeed("io"))
.orElse {
// Fallback if reflection fails, user can wrap in IO(...)
self.findPort("io")
.collect { case r: Record => r }
}.getOrElse(throwException(
s"Compatibility mode Module '$this' must have a 'val io' Bundle. " +
"If there is such a field and you still see this error, autowrapping has failed (sorry!). " +
"Please wrap the Bundle declaration in IO(...)."
))
}

/** Legacy Module class that restricts IOs to just io, clock, and reset, and provides a constructor
* for threading through explicit clock and reset.
*
* While this class isn't planned to be removed anytime soon (there are benefits to restricting
* IO), the clock and reset constructors will be phased out. Recommendation is to wrap the module
* in a withClock/withReset/withClockAndReset block, or directly hook up clock or reset IO pins.
* This is only intended as a bridge from chisel3 to Chisel.Module, do not extend this in user
* code, use [[Module]]
*/
abstract class LegacyModule(implicit moduleCompileOptions: CompileOptions) extends Module {
// IO for this Module. At the Scala level (pre-FIRRTL transformations),
// connections in and out of a Module may only go through `io` elements.
@deprecated("Removed for causing issues in Scala 2.12+. You remain free to define io Bundles " +
"in your Modules, but you cannot rely on an io field in every Module. " +
"For more information, see: https://github.com/freechipsproject/chisel3/pull/1550.",
"Chisel 3.4"
)
def io: Record

// Private accessor to reduce number of deprecation warnings
private[chisel3] def _io: Record = io

private lazy val _io: Record = reflectivelyFindValIO(this)

// Allow access to bindings from the compatibility package
protected def _compatIoPortBound() = portsContains(_io)

private[chisel3] override def namePorts(names: HashMap[HasId, String]): Unit = {
for (port <- getModulePorts) {
// This should already have been caught
if (!names.contains(port)) throwException(s"Unable to name port $port in $this")
val name = names(port)
port.setRef(ModuleIO(this, _namespace.name(name)))
}
}

private[chisel3] override def generateComponent(): Component = {
_compatAutoWrapPorts() // pre-IO(...) compatibility hack

Expand All @@ -195,5 +218,33 @@ package internal {

super.generateComponent()
}

override def _compatAutoWrapPorts(): Unit = {
if (!_compatIoPortBound() && _io != null) {
_bindIoInPlace(_io)
}
}
}

import chisel3.experimental.Param

/** Legacy BlackBox class will reflectively autowrap val io
*
* '''Do not use this class in user code'''. Use whichever `Module` is imported by your wildcard
* import (preferably `import chisel3._`).
*/
abstract class LegacyBlackBox(params: Map[String, Param] = Map.empty[String, Param])
(implicit moduleCompileOptions: CompileOptions)
extends chisel3.BlackBox(params) {

override private[chisel3] lazy val _io: Record = reflectivelyFindValIO(this)

// This class auto-wraps the BlackBox with IO(...), allowing legacy code (where IO(...) wasn't
// required) to build.
override def _compatAutoWrapPorts(): Unit = {
if (!_compatIoPortBound()) {
_bindIoInPlace(_io)
}
}
}
}
17 changes: 1 addition & 16 deletions src/main/scala/chisel3/compatibility.scala
Original file line number Diff line number Diff line change
Expand Up @@ -259,16 +259,7 @@ package object Chisel {

implicit def resetToBool(reset: Reset): Bool = reset.asBool

import chisel3.experimental.Param
abstract class BlackBox(params: Map[String, Param] = Map.empty[String, Param]) extends chisel3.BlackBox(params) {
// This class auto-wraps the BlackBox with IO(...), allowing legacy code (where IO(...) wasn't
// required) to build.
override def _compatAutoWrapPorts(): Unit = {
if (!_compatIoPortBound()) {
_bindIoInPlace(io)
}
}
}
type BlackBox = chisel3.internal.LegacyBlackBox

type MemBase[T <: Data] = chisel3.MemBase[T]

Expand Down Expand Up @@ -321,12 +312,6 @@ package object Chisel {
this(None, Option(_reset))(moduleCompileOptions)
def this(_clock: Clock, _reset: Bool)(implicit moduleCompileOptions: CompileOptions) =
this(Option(_clock), Option(_reset))(moduleCompileOptions)

override def _compatAutoWrapPorts(): Unit = {
if (!_compatIoPortBound() && io != null) {
_bindIoInPlace(io)
}
}
}

val Module = chisel3.Module
Expand Down
4 changes: 4 additions & 0 deletions src/test/scala/chiselTests/AnalogIntegrationSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@ class AnalogBlackBox(index: Int) extends BlackBox(Map("index" -> index)) {
val io = IO(new AnalogBlackBoxIO(1))
}

// This interface exists to give a common interface type for AnalogBlackBoxModule and
// AnalogBlackBoxWrapper. This is the standard way to deal with the deprecation and removal of the
// Module.io virtual method (same for BlackBox.io).
// See https://github.com/freechipsproject/chisel3/pull/1550 for more information
trait AnalogBlackBoxModuleIntf extends Module {
def io: AnalogBlackBoxIO
}
Expand Down
28 changes: 28 additions & 0 deletions src/test/scala/chiselTests/BlackBox.scala
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ class BlackBoxInverter extends BlackBox {
})
}

// Due to the removal of "val io", this technically works
// This style is discouraged, please use "val io"
class BlackBoxInverterSuggestName extends BlackBox {
override def desiredName: String = "BlackBoxInverter"
val foo = IO(new Bundle() {
val in = Input(Bool())
val out = Output(Bool())
}).suggestName("io")
}

class BlackBoxPassthrough extends BlackBox {
val io = IO(new Bundle() {
val in = Input(Bool())
Expand Down Expand Up @@ -50,6 +60,18 @@ class BlackBoxTester extends BasicTester {
stop()
}

class BlackBoxTesterSuggestName extends BasicTester {
val blackBoxPos = Module(new BlackBoxInverterSuggestName)
val blackBoxNeg = Module(new BlackBoxInverterSuggestName)

blackBoxPos.foo.in := 1.U
blackBoxNeg.foo.in := 0.U

assert(blackBoxNeg.foo.out === 1.U)
assert(blackBoxPos.foo.out === 0.U)
stop()
}

class BlackBoxFlipTester extends BasicTester {
val blackBox = Module(new BlackBoxPassthrough2)

Expand Down Expand Up @@ -187,4 +209,10 @@ class BlackBoxSpec extends ChiselFlatSpec {
}
)
}
"A BlackBoxed using suggestName(\"io\")" should "work (but don't do this)" in {
assertTesterPasses(
{new BlackBoxTesterSuggestName},
Seq("/chisel3/BlackBoxTest.v"),
TesterDriver.verilatorOnly)
}
}
14 changes: 14 additions & 0 deletions src/test/scala/chiselTests/CompatibilitySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,20 @@ class CompatibiltySpec extends ChiselFlatSpec with ScalaCheckDrivenPropertyCheck
ChiselStage.elaborate { new RequireIOWrapModule() }
}

"A Module without val io" should "throw an exception" in {
class ModuleWithoutValIO extends Module {
val foo = new Bundle {
val in = UInt(width = 32).asInput
val out = Bool().asOutput
}
foo.out := foo.in(1)
}
val e = intercept[Exception](
ChiselStage.elaborate { new ModuleWithoutValIO }
)
e.getMessage should include("must have a 'val io' Bundle")
}

"A Module connecting output as source to input as sink when compiled with the Chisel compatibility package" should "not throw an exception" in {

class SimpleModule extends Module {
Expand Down

0 comments on commit 8a73362

Please sign in to comment.