Skip to content

Commit

Permalink
Merge branch '3.2.x' into 3.2-release
Browse files Browse the repository at this point in the history
  • Loading branch information
ucbjrl committed Apr 22, 2020
2 parents 38d79f9 + 32ce3bc commit 98f78a1
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 49 deletions.
2 changes: 1 addition & 1 deletion build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ lazy val chisel = (project in file(".")).
// sufficient to suppress subproject JAR creation, so we can restore
// general aggregation, and thus get coverage tests and scaladoc for subprojects.
aggregate(coreMacros, chiselFrontend).
settings(mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3" % "3.2.0")).
settings(mimaPreviousArtifacts := Set("edu.berkeley.cs" %% "chisel3" % "3.2.4")).
settings(mimaCurrentClassfiles := (packageBin in Compile).value).
settings(
scalacOptions in Test ++= Seq("-language:reflectiveCalls"),
Expand Down
37 changes: 20 additions & 17 deletions chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,12 @@ private[chisel3] object Builder {
dynamicContextVar.value.get
}

private val chiselContext = new DynamicVariable[ChiselContext](new ChiselContext)
// Ensure we have a thread-specific ChiselContext
private val chiselContext = new ThreadLocal[ChiselContext]{
override def initialValue: ChiselContext = {
new ChiselContext
}
}

// Initialize any singleton objects before user code inadvertently inherits them.
private def initializeSingletons(): Unit = {
Expand All @@ -249,7 +254,7 @@ private[chisel3] object Builder {

def namingStackOption: Option[NamingStack] = dynamicContextVar.value.map(_.namingStack)

def idGen: IdGen = chiselContext.value.idGen
def idGen: IdGen = chiselContext.get.idGen

def globalNamespace: Namespace = dynamicContext.globalNamespace
def components: ArrayBuffer[Component] = dynamicContext.components
Expand Down Expand Up @@ -350,18 +355,18 @@ private[chisel3] object Builder {
// Prune the existing Bundle stack of closed Bundles
// If we know where we are in the stack, discard frames above that
val stackEltsTop = if (eltStackPos >= 0) eltStackPos else stackElts.size
val pruneLength = chiselContext.value.bundleStack.reverse.prefixLength { case (_, cname, mname, pos) =>
val pruneLength = chiselContext.get.bundleStack.reverse.prefixLength { case (_, cname, mname, pos) =>
pos >= stackEltsTop || stackElts(pos).getClassName != cname || stackElts(pos).getMethodName != mname
}
chiselContext.value.bundleStack.trimEnd(pruneLength)
chiselContext.get.bundleStack.trimEnd(pruneLength)

// Return the stack state before adding the most recent bundle
val lastStack = chiselContext.value.bundleStack.map(_._1).toSeq
val lastStack = chiselContext.get.bundleStack.map(_._1).toSeq

// Append the current Bundle to the stack, if it's on the stack trace
if (eltStackPos >= 0) {
val stackElt = stackElts(eltStackPos)
chiselContext.value.bundleStack.append((elt, eltClassName, stackElt.getMethodName, eltStackPos))
chiselContext.get.bundleStack.append((elt, eltClassName, stackElt.getMethodName, eltStackPos))
}
// Otherwise discard the stack frame, this shouldn't fail noisily

Expand Down Expand Up @@ -400,17 +405,15 @@ private[chisel3] object Builder {
}

def build[T <: RawModule](f: => T): (Circuit, T) = {
chiselContext.withValue(new ChiselContext) {
dynamicContextVar.withValue(Some(new DynamicContext())) {
errors.info("Elaborating design...")
val mod = f
mod.forceName(mod.name, globalNamespace)
errors.checkpoint()
errors.info("Done elaborating.")

(Circuit(components.last.name, components, annotations), mod)
}
}
dynamicContextVar.withValue(Some(new DynamicContext())) {
errors.info("Elaborating design...")
val mod = f
mod.forceName(mod.name, globalNamespace)
errors.checkpoint()
errors.info("Done elaborating.")

(Circuit(components.last.name, components, annotations), mod)
}
}
initializeSingletons()
}
Expand Down
61 changes: 52 additions & 9 deletions chiselFrontend/src/main/scala/chisel3/internal/Error.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,77 @@

package chisel3.internal

import scala.annotation.tailrec
import scala.collection.mutable.{ArrayBuffer, LinkedHashMap}

class ChiselException(message: String, cause: Throwable = null) extends Exception(message, cause) {

val blacklistPackages = Set("chisel3", "scala", "java", "sun", "sbt")
val builderName = "chisel3.internal.Builder"
/** Package names whose stack trace elements should be trimmed when generating a trimmed stack trace */
val blacklistPackages: Set[String] = Set("chisel3", "scala", "java", "sun", "sbt")

/** trims the top of the stack of elements belonging to [[blacklistPackages]]
* then trims the bottom elements until it reaches [[builderName]]
* then continues trimming elements belonging to [[blacklistPackages]]
/** The object name of Chisel's internal `Builder`. Everything stack trace element after this will be trimmed. */
val builderName: String = chisel3.internal.Builder.getClass.getName

/** Examine a [[Throwable]], to extract all its causes. Innermost cause is first.
* @param throwable an exception to examine
* @return a sequence of all the causes with innermost cause first
*/
@tailrec
private def getCauses(throwable: Throwable, acc: Seq[Throwable] = Seq.empty): Seq[Throwable] =
throwable.getCause() match {
case null => throwable +: acc
case a => getCauses(a, throwable +: acc)
}

/** Returns true if an exception contains */
private def containsBuilder(throwable: Throwable): Boolean =
throwable.getStackTrace().collectFirst {
case ste if ste.getClassName().startsWith(builderName) => throwable
}.isDefined

/** Examine this [[ChiselException]] and it's causes for the first [[Throwable]] that contains a stack trace including
* a stack trace element whose declaring class is the [[builderName]]. If no such element exists, return this
* [[ChiselException]].
*/
def trimmedStackTrace: Array[StackTraceElement] = {
private lazy val likelyCause: Throwable =
getCauses(this).collectFirst{ case a if containsBuilder(a) => a }.getOrElse(this)

/** For an exception, return a stack trace trimmed to user code only
*
* This does the following actions:
*
* 1. Trims the top of the stack trace while elements match [[blacklistPackages]]
* 2. Trims the bottom of the stack trace until an element matches [[builderName]]
* 3. Trims from the [[builderName]] all [[blacklistPackages]]
*
* @param throwable the exception whose stack trace should be trimmed
* @return an array of stack trace elements
*/
private def trimmedStackTrace(throwable: Throwable): Array[StackTraceElement] = {
def isBlacklisted(ste: StackTraceElement) = {
val packageName = ste.getClassName().takeWhile(_ != '.')
blacklistPackages.contains(packageName)
}

val trimmedLeft = getStackTrace().view.dropWhile(isBlacklisted)
val trimmedLeft = throwable.getStackTrace().view.dropWhile(isBlacklisted)
val trimmedReverse = trimmedLeft.reverse
.dropWhile(ste => !ste.getClassName.startsWith(builderName))
.dropWhile(isBlacklisted)
trimmedReverse.reverse.toArray
}

/** trims the top of the stack of elements belonging to [[blacklistPackages]]
* then trims the bottom elements until it reaches [[builderName]]
* then continues trimming elements belonging to [[blacklistPackages]]
*/
@deprecated("This method will be removed in 3.4", "3.3")
def trimmedStackTrace: Array[StackTraceElement] = trimmedStackTrace(this)

def chiselStackTrace: String = {
val trimmed = trimmedStackTrace
val trimmed = trimmedStackTrace(likelyCause)

val sw = new java.io.StringWriter
sw.write(toString + "\n")
sw.write(likelyCause.toString + "\n")
sw.write("\t...\n")
trimmed.foreach(ste => sw.write(s"\tat $ste\n"))
sw.write("\t... (Stack trace trimmed to user code only, rerun with --full-stacktrace if you wish to see the full stack trace)\n") // scalastyle:ignore line.size.limit
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/chisel3/compatibility.scala
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ package object Chisel { // scalastyle:ignore package.object.name number.of.t

import chisel3.CompileOptions
abstract class CompatibilityModule(implicit moduleCompileOptions: CompileOptions)
extends chisel3.internal.LegacyModule {
extends chisel3.internal.LegacyModule()(moduleCompileOptions) {
// This class auto-wraps the Module IO with IO(...), allowing legacy code (where IO(...) wasn't
// required) to build.
// Also provides the clock / reset constructors, which were used before withClock happened.
Expand Down
2 changes: 1 addition & 1 deletion src/main/scala/chisel3/stage/ChiselAnnotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ case class ChiselGeneratorAnnotation(gen: () => RawModule) extends NoTargetAnnot
} catch {
case e @ (_: OptionsException | _: ChiselException) => throw e
case e: Throwable =>
throw new OptionsException(s"Exception thrown when elaborating ChiselGeneratorAnnotation", e)
throw new ChiselException(s"Exception thrown when elaborating ChiselGeneratorAnnotation", e)
}

}
Expand Down
22 changes: 9 additions & 13 deletions src/main/scala/chisel3/util/Counter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,17 @@ import chisel3.internal.naming.chiselName // can't use chisel3_ version because
*/
@chiselName
class Counter(val n: Int) {
require(n >= 0)
require(n >= 0, s"Counter value must be nonnegative, got: $n")
val value = if (n > 1) RegInit(0.U(log2Ceil(n).W)) else 0.U

/** Increment the counter, returning whether the counter currently is at the
* maximum and will wrap. The incremented value is registered and will be
* visible on the next cycle.
/** Increment the counter
*
* @note The incremented value is registered and will be visible on the next clock cycle
* @return whether the counter will wrap to zero on the next cycle
*/
def inc(): Bool = {
if (n > 1) {
val wrap = value === (n-1).asUInt
val wrap = value === (n-1).U
value := value + 1.U
if (!isPow2(n)) {
when (wrap) { value := 0.U }
Expand Down Expand Up @@ -62,13 +63,8 @@ object Counter
@chiselName
def apply(cond: Bool, n: Int): (UInt, Bool) = {
val c = new Counter(n)
// Note this use of var is generally frowned upon! Don't use this as an example.
// This is done because we wanted the hardware generated by c.inc() to be wrapped
// in a when statement, but still needed to refer to the final value returned by
// c.inc() so we could return cond && wrap. Unless you really, really know what
// you are doing, IGNORE THIS CODE!!!!!
var wrap: Bool = null
when (cond) { wrap = c.inc() }
(c.value, cond && wrap)
val wrap = WireInit(false.B)
when (cond) { wrap := c.inc() }
(c.value, wrap)
}
}
5 changes: 2 additions & 3 deletions src/test/scala/chiselTests/ChiselSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import org.scalatest.prop._
import org.scalacheck._
import chisel3._
import chisel3.testers._
import firrtl.options.OptionsException
import firrtl.{AnnotationSeq, CommonOptions, ExecutionOptionsManager, FirrtlExecutionFailure, FirrtlExecutionSuccess, HasFirrtlOptions}
import firrtl.util.BackendCompilationUtilities
import java.io.ByteArrayOutputStream
Expand Down Expand Up @@ -100,7 +99,7 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
import org.scalatest.exceptions.TestFailedException
// Who tests the testers?
"assertKnownWidth" should "error when the expected width is wrong" in {
val caught = intercept[OptionsException] {
val caught = intercept[ChiselException] {
assertKnownWidth(7) {
Wire(UInt(8.W))
}
Expand All @@ -123,7 +122,7 @@ class ChiselTestUtilitiesSpec extends ChiselFlatSpec {
}

"assertInferredWidth" should "error if the width is known" in {
val caught = intercept[OptionsException] {
val caught = intercept[ChiselException] {
assertInferredWidth(8) {
Wire(UInt(8.W))
}
Expand Down
17 changes: 17 additions & 0 deletions src/test/scala/chiselTests/CompatibilitySpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import chisel3.testers.BasicTester
import org.scalacheck.Gen
import org.scalatest.prop.GeneratorDrivenPropertyChecks

// Need separate import to override compile options from Chisel._
object CompatibilityCustomCompileOptions {
import Chisel.{defaultCompileOptions => _, _}
implicit val customCompileOptions =
chisel3.ExplicitCompileOptions.NotStrict.copy(inferModuleReset = true)
class Foo extends Module {
val io = new Bundle {}
}
}

class CompatibiltySpec extends ChiselFlatSpec with GeneratorDrivenPropertyChecks {
import Chisel._

Expand Down Expand Up @@ -581,4 +591,11 @@ class CompatibiltySpec extends ChiselFlatSpec with GeneratorDrivenPropertyChecks
elaborate(new Foo)
}

it should "properly propagate custom compileOptions in Chisel.Module" in {
import CompatibilityCustomCompileOptions._
var result: Foo = null
elaborate({result = new Foo; result})
result.compileOptions should be theSameInstanceAs (customCompileOptions)
}

}
16 changes: 12 additions & 4 deletions src/test/scala/chiselTests/OneHotMuxSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ package chiselTests

import chisel3._
import chisel3.experimental.FixedPoint
import chisel3.internal.ChiselException
import chisel3.testers.BasicTester
import chisel3.util.{Mux1H, UIntToOH}
import org.scalatest._
Expand Down Expand Up @@ -31,23 +32,31 @@ class OneHotMuxSpec extends FreeSpec with Matchers with ChiselRunners {
assertTesterPasses(new ParameterizedAggregateOneHotTester)
}
"simple one hot mux with all aggregates containing inferred width fixed values should NOT work" in {
intercept[ChiselException] {
intercept [ChiselException] {
assertTesterPasses(new InferredWidthAggregateOneHotTester)
}
}
"simple one hot mux with all fixed width bundles but with different bundles should Not work" in {
intercept[IllegalArgumentException] {
try {
assertTesterPasses(new DifferentBundleOneHotTester)
} catch {
case a: ChiselException => a.getCause match {
case _: IllegalArgumentException =>
}
}
}
"UIntToOH with output width greater than 2^(input width)" in {
assertTesterPasses(new UIntToOHTester)
}
"UIntToOH should not accept width of zero (until zero-width wires are fixed" in {
intercept[java.lang.IllegalArgumentException] {
try {
assertTesterPasses(new BasicTester {
val out = UIntToOH(0.U, 0)
})
} catch {
case a: ChiselException => a.getCause match {
case _: IllegalArgumentException =>
}
}
}

Expand Down Expand Up @@ -305,4 +314,3 @@ class UIntToOHTester extends BasicTester {

stop()
}

18 changes: 18 additions & 0 deletions src/test/scala/chiselTests/stage/ChiselMainSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ object ChiselMainSpec {
out := in
}

/** A module that fails a requirement */
class FailingRequirementModule extends RawModule {
require(false)
}

}

class ChiselMainSpec extends FeatureSpec with GivenWhenThen with Matchers with chiselTests.Utils {
Expand Down Expand Up @@ -114,4 +119,17 @@ class ChiselMainSpec extends FeatureSpec with GivenWhenThen with Matchers with c
).foreach(runStageExpectFiles)
}

feature("Report properly trimmed stack traces") {
Seq(
ChiselMainTest(args = Array("-X", "low"),
generator = Some(classOf[FailingRequirementModule]),
stdout = Some("requirement failed"),
result = 1),
ChiselMainTest(args = Array("-X", "low", "--full-stacktrace"),
generator = Some(classOf[FailingRequirementModule]),
stdout = Some("chisel3.internal.ChiselException"),
result = 1)
).foreach(runStageExpectFiles)
}

}

0 comments on commit 98f78a1

Please # to comment.