Skip to content

Commit

Permalink
Merge branch '3.1.x' into 3.1.0
Browse files Browse the repository at this point in the history
  • Loading branch information
ucbjrl committed Apr 10, 2018
2 parents 10751f5 + 01d10fa commit f1cc75f
Show file tree
Hide file tree
Showing 8 changed files with 361 additions and 21 deletions.
4 changes: 2 additions & 2 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ def javacOptionsVersion(scalaVersion: String): Seq[String] = {
}
}

val defaultVersions = Map("firrtl" -> "1.1.0-RC1")
val defaultVersions = Map("firrtl" -> "1.1.0-RC2")

lazy val commonSettings = Seq (
organization := "edu.berkeley.cs",
version := "3.1.0-RC1",
version := "3.1.0-RC2",
git.remoteRepo := "git@github.com:freechipsproject/chisel3.git",
autoAPIMappings := true,
scalaVersion := "2.11.12",
Expand Down
24 changes: 19 additions & 5 deletions chiselFrontend/src/main/scala/chisel3/core/Aggregate.scala
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,10 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
case (outerClass, _) => Some(outerClass)
}

// For compatibility with pre-3.1, where null is tried as an argument to the constructor.
// This stores potential error messages which may be used later.
var outerClassError: Option[String] = None

// Check if this is an inner class, and if so, try to get the outer instance
val outerClassInstance = enclosingClassOption.map { outerClass =>
def canAssignOuterClass(x: Object) = outerClass.isAssignableFrom(x.getClass)
Expand All @@ -612,10 +616,14 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
case outer :: Nil =>
_outerInst = Some(outer) // record the guess for future use
outer
case Nil => autoClonetypeError(s"Unable to determine instance of outer class $outerClass," +
s" no candidates assignable to outer class types; examined $allOuterCandidates")
case candidates => autoClonetypeError(s"Unable to determine instance of outer class $outerClass," +
s" multiple possible candidates $candidates assignable to outer class type")
case Nil => // TODO: replace with fatal autoClonetypeError once compatibility period is dropped
outerClassError = Some(s"Unable to determine instance of outer class $outerClass," +
s" no candidates assignable to outer class types; examined $allOuterCandidates")
null
case candidates => // TODO: replace with fatal autoClonetypeError once compatibility period is dropped
outerClassError = Some(s"Unable to determine instance of outer class $outerClass," +
s" multiple possible candidates $candidates assignable to outer class type")
null
}
}
}
Expand All @@ -635,7 +643,12 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {
case (Nil, None) => // no arguments, no outer class, invoke constructor directly
Some(ctor.newInstance().asInstanceOf[this.type])
case (argType :: Nil, Some((_, outerInstance))) =>
if (argType isAssignableFrom outerInstance.getClass) {
if (outerInstance == null) {
Builder.deprecated(s"chisel3.1 autoclonetype failed, falling back to 3.0 behavior using null as the outer instance." +
s" Autoclonetype failure reason: ${outerClassError.get}",
Some(s"$clazz"))
Some(ctor.newInstance(outerInstance).asInstanceOf[this.type])
} else if (argType isAssignableFrom outerInstance.getClass) {
Some(ctor.newInstance(outerInstance).asInstanceOf[this.type])
} else {
None
Expand Down Expand Up @@ -738,6 +751,7 @@ abstract class Bundle(implicit compileOptions: CompileOptions) extends Record {

// Invoke ctor
val classMirror = outerClassInstance match {
case Some((_, null)) => autoClonetypeError(outerClassError.get) // deals with the null hack for 3.0 compatibility
case Some((_, outerInstance)) => mirror.reflect(outerInstance).reflectClass(classSymbol)
case _ => mirror.reflectClass(classSymbol)
}
Expand Down
1 change: 1 addition & 0 deletions chiselFrontend/src/main/scala/chisel3/core/Mem.scala
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ sealed class SyncReadMem[T <: Data](t: T, n: Int) extends MemBase[T](t, n) {

def do_read(addr: UInt, enable: Bool)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): T = {
val a = Wire(UInt())
a := DontCare
var port: Option[T] = None
when (enable) {
a := addr
Expand Down
23 changes: 14 additions & 9 deletions chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ private[chisel3] class DynamicContext() {
var currentClockAndReset: Option[ClockAndReset] = None
val errors = new ErrorLog
val namingStack = new internal.naming.NamingStack
// Record the Bundle instance, class name, and reverse stack trace position of open Bundles
val bundleStack: ArrayBuffer[(Bundle, String, Int)] = ArrayBuffer()
// Record the Bundle instance, class name, method name, and reverse stack trace position of open Bundles
val bundleStack: ArrayBuffer[(Bundle, String, String, Int)] = ArrayBuffer()
}

private[chisel3] object Builder {
Expand Down Expand Up @@ -247,24 +247,29 @@ private[chisel3] object Builder {
// Returns the current stack of open Bundles
// Note: elt will NOT have finished construction, its elements cannot be accessed
def updateBundleStack(elt: Bundle): Seq[Bundle] = {
val stackClasses = Thread.currentThread().getStackTrace()
.map(_.getClassName)
val stackElts = Thread.currentThread().getStackTrace()
.reverse // so stack frame numbers are deterministic across calls
.dropRight(2) // discard Thread.getStackTrace and updateBundleStack

// Determine where we are in the Bundle stack
val eltClassName = elt.getClass.getName
val eltStackPos = stackElts.map(_.getClassName).lastIndexOf(eltClassName)

// Prune the existing Bundle stack of closed Bundles
val pruneLength = dynamicContext.bundleStack.reverse.prefixLength { case (_, cname, pos) =>
pos >= stackClasses.size || stackClasses(pos) != cname
// If we know where we are in the stack, discard frames above that
val stackEltsTop = if (eltStackPos >= 0) eltStackPos else stackElts.size
val pruneLength = dynamicContext.bundleStack.reverse.prefixLength { case (_, cname, mname, pos) =>
pos >= stackEltsTop || stackElts(pos).getClassName != cname || stackElts(pos).getMethodName != mname
}
dynamicContext.bundleStack.trimEnd(pruneLength)

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

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

Expand Down
276 changes: 276 additions & 0 deletions doc/style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,276 @@
# Chisel Style Guide

The Chisel style guide reflects the [Google Java style
guide](http://https://google.github.io/styleguide/javaguide.html) and the [General Public Scala style
guide](http://docs.scala-lang.org/style/). Specific rules below are to clarify
the style used for the chisel3 repo and repos related to Chisel (Firrtl).

**Goal:** Readability and consistency are the main purposes of the style guide.
Writing your code so someone else (or yourself) can grok it later is important
to code health and quality.

## Filenames
The source file name consists of the case-sensitive name of the top-level class
it contains, plus ".scala".

## Packages

Package definitions must contain the full path to the package from scala. If
you create a subpackage, it should go in a subdirectory.

package directory.name.to.get.you.to.your.source

As in Scala, packages follow the [Java package naming convention](https://google.github.io/styleguide/javaguide.html#s5.2.1-package-names).
Note that these guidelines call for all lowercase, no underscores.

```scala
// Do this
package hardware.chips.topsecret.masterplan

// Not this
package hardware.chips.veryObvious.bad_style
```

We also suggest you do not use chisel3 as a package, and especially do not use it
as the final (innermost) package.

```scala
// Don't do this
package hardware.chips.newchip.superfastcomponent.chisel3

// This will lead to instantiating package members like so:
val module = Module(new chisel3.FastModule)

// Which collides with the chisel namespace
import chisel3._
```

## Imports
Avoid wildcard ( ._ ) imports, with the exception of chisel3._
All other imports must call out used methods.
import chisel3._ must be first and separated from remaining imports with extra
blank line.

**Reason:** This makes it clear where methods are defined.

Remaining imports must be listed alphabetically.

```scala
import chisel3._

import the.other.thing.that.i.reference.inline
import the.other.things.that.i.reference.{ClassOne, ClassTwo}


val myInline = inline.MakeAnInline()
val myClassOne = new ClassOne
```

## Tests
Test classes are named starting with the name of the class they are testing, and
ending with "Test".
Test files must start with the name of the class you are testing and end with
"Test.scala".
Test files should reside in a subdirectory called "tests".
The tests package should be composed of the package class you are testing.

```scala
package class.under.test.class
package tests
```

## Comments
We use scaladoc to automatically generate documentation from the source code.

```scala
/** Multiple lines of ScalaDoc text are written here,
* wrapped normally...
*/
public int method(String p1) { ... }
```

... or in this single-line example:

```scala
/** An especially short bit of Javadoc. */
```

Write documentation as if the person reading it knows more about Scala and
Chisel than you. If you find comments in the code consider breaking them up
into seperate methods.

## Module Classes and Instances

Modules can take different forms in Chisel. First, in the verilog sense where
you instance the module and then hook it up. In this case Module(new MyMod()) is
returning a reference to the module.

```scala
val myMod = Module(new MyMod())
myMod.io <> hookUp
```

Second, in a more programmatic inline style with factory methods. In this case
Queue is actually returning the part of the IO bundle representing the queue's
output. The factory method takes the input IO to the queue and an optional param
for depth.

```scala
val queueOut = Queue(queueIn, depth=10)
```

The latter can be used for composing multiple functions into a single line.

```scala
val queueOut = Queue(
Arbitrate.byRoundRobin(
Queue(a), // depth assumed to be 1
Queue(b, depth=3),
Queue(c, depth=4)
),
depth=10
)
```

## Naming Conventions

Chisel follows the [Scala Naming Conventions](http://docs.scala-lang.org/style/naming-conventions.html).
In general, Chisel code should use CamelCase for naming (ie. the first letter
of each word is capitalized except sometimes the first word).

### Why CamelCase instead of Snake\_Case?

The compiler inserts underscores when splitting Chisel/FIRRTL aggregate types
into Verilog types. The compiler uses underscores to preserve the original
structure of the data in the resulting Verilog. Because of the special meaning
of underscores in Chisel-generated Verilog, their use in naming is **strongly**
discouraged.

Consider the following Chisel code:

```scala
val msg = Wire(new Bundle {
val valid = Bool()
val addr = UInt(32)
val data = UInt(64)
})
val msg_rec = Wire(Bool())
```

Which compiles to the Verilog:

```verilog
wire msg_valid;
wire [31:0] msg_addr;
wire [63:0] msg_data;
wire msg_rec;
```

The Verilog maintains the structure of the original aggregate wire `msg`.
However, because we named another variable `msg_rec`, it appears in the Verilog
as if `msg` had 4 fields instead of its actual 3! If we instead follow the
lowerCamelCase for values naming convention, the resulting Verilog makes more
sense:

```scala
val msg = Wire(new Bundle {
val valid = Bool()
val addr = UInt(32)
val data = UInt(64)
})
val msgRec = Wire(Bool())
```

And its resulting Verilog:

```verilog
wire msg_valid;
wire [31:0] msg_addr;
wire [63:0] msg_data;
wire msgRec;
```

Much better.

### Modules and Bundles (Classes, Traits, and Objects)

Modules are Scala classes and thus use UpperCamelCase.

```scala
class ModuleNamingExample extends Module {
...
}
```

Similarly, other classes (Chisel & Scala) should be UpperCamelCase as well.

```scala
trait UsefulScalaUtilities {
def isEven(n: Int): Boolean = (n % 2) == 0
def isOdd(n: Int): Boolean = !isEven(n)
}

class MyCustomBundle extends Bundle {
...
}
// Companion object to MyCustomBundle
object MyCustomBundle {
...
}

```

### Values and Methods

Values and methods should use lowerCamelCase. (Unless the value is a constant.)

```scala
val mySuperReg = Reg(init = 0.asUInt(32))
def myImportantMethod(a: UInt): Bool = a < 23.asUInt
```

### Constants

Unlike the Google Java style, constants use UpperCamelCase, which is in line
with the official [Scala Naming
Conventions](https://docs.scala-lang.org/style/naming-conventions.html).
Constants are final fields (val or object) whose contents are deeply immutable
and belong to a package object or an object. Examples:

```scala
// Constants
object Constants {
val Number = 5
val Names = "Ed" :: "Ann" :: Nil
val Ages = Map("Ed" -> 35, "Ann" -> 32)
}

// Not constants
class NonConstantsInClass {
val inClass: String = "in-class"
}

object nonConstantsInObject {
var varString = "var-string"
val mutableCollection: scala.collection.mutable.Set[String]
val mutableElements = Set(mutable)
}
```

### UpperCamelCase vs. lowerCamelCase

There is more than one reasonable way to covert English prose into camel case.
We follow the convention defined in the [Google Java style
guide](https://google.github.io/styleguide/javaguide.html#s5.3-camel-case). The
potentially non-obvious rule being to treat acronymns as words for the purpose
of camel case.

Note that the casing of the original words is almost entirely disregarded.
Example:

Prose form | UpperCamelCase | lowerCamelCase | Incorrect
:------------- | :------------- | :------------- | :------------
find GCD | FindGcd | findGcd | ~~findGCD~~
state for FSM | StateForFsm | stateForFsm | ~~stateForFSM~~
mock dut | MockDut | mockDut | ~~MockDUT~~
FIFO Generator | FifoGenerator | fifoGenerator | ~~FIFOGenerator~~
Loading

0 comments on commit f1cc75f

Please sign in to comment.