From 3d0209c3efa2bb5882fe12843aed281bd937c0e1 Mon Sep 17 00:00:00 2001 From: chick Date: Wed, 1 Apr 2020 10:26:14 -0700 Subject: [PATCH 1/4] Treadle clean up. Fix doc warnings. Add fatal warnings flag Cleanup unused imports Fixed warning on source not closed --- build.sbt | 8 +++- build.sc | 4 +- src/main/scala/treadle/ScalaBlackBox.scala | 2 +- src/main/scala/treadle/TreadleOptions.scala | 6 +-- src/main/scala/treadle/TreadleTester.scala | 5 +- .../treadle/executable/DataStorePlugIn.scala | 4 -- .../scala/treadle/executable/Memory.scala | 31 +++++++------ .../treadle/executable/RollBackBuffer.scala | 2 - .../treadle/executable/Snapshotter.scala | 10 ---- .../scala/treadle/executable/Symbol.scala | 4 +- src/main/scala/treadle/package.scala | 18 ++++++-- src/main/scala/treadle/repl/ReplOptions.scala | 1 - .../scala/treadle/repl/TreadleReplCli.scala | 1 - .../scala/treadle/stage/TreadleStage.scala | 7 ++- .../treadle/stage/phases/PrepareAst.scala | 8 ++-- .../scala/treadle/utils/RemoveTempWires.scala | 46 +++++++++---------- 16 files changed, 76 insertions(+), 81 deletions(-) diff --git a/build.sbt b/build.sbt index 41d15759..153e19e5 100644 --- a/build.sbt +++ b/build.sbt @@ -133,7 +133,11 @@ publishTo := { // scalacOptions in Compile ++= Seq( "-deprecation", - "-unchecked" + "-unchecked", + "-language:reflectiveCalls", + "-language:existentials", + "-language:implicitConversions", + "-Ywarn-unused-import" // required by `RemoveUnused` rule ) // @@ -141,6 +145,8 @@ scalacOptions in Compile ++= Seq( // scalacOptions in Compile in doc ++= Seq( "-deprecation", + "-Xfatal-warnings", + "-feature", "-diagrams", "-diagrams-max-classes", "25", "-doc-version", version.value, diff --git a/build.sc b/build.sc index 82e6b43a..e54a0850 100644 --- a/build.sc +++ b/build.sc @@ -29,7 +29,9 @@ trait CommonModule extends CrossUnRootedSbtModule with PublishModule { override def scalacOptions = Seq( "-deprecation", "-explaintypes", - "-feature", "-language:reflectiveCalls", + "-Xfatal-warnings", + "-feature", + "-language:reflectiveCalls", "-unchecked", "-Xcheckinit", "-Xlint:infer-any", diff --git a/src/main/scala/treadle/ScalaBlackBox.scala b/src/main/scala/treadle/ScalaBlackBox.scala index eac10f25..b238da56 100644 --- a/src/main/scala/treadle/ScalaBlackBox.scala +++ b/src/main/scala/treadle/ScalaBlackBox.scala @@ -17,7 +17,7 @@ limitations under the License. package treadle import firrtl.ir.{Param, Type} -import treadle.executable.{DataStore, Symbol, Transition} +import treadle.executable.Transition import scala.collection._ diff --git a/src/main/scala/treadle/TreadleOptions.scala b/src/main/scala/treadle/TreadleOptions.scala index 92f7da0a..6ca9a9b7 100644 --- a/src/main/scala/treadle/TreadleOptions.scala +++ b/src/main/scala/treadle/TreadleOptions.scala @@ -283,7 +283,7 @@ object TreadleFirrtlFormHint extends HasShellOptions { case class TreadleTesterAnnotation(tester: TreadleTester) extends NoTargetAnnotation with TreadleOption /** - * Factory for [[FirrtlSourceAnnotation]], this is an alias for FirrtlCli + * Factory for FirrtlSourceAnnotation, this is an alias for FirrtlCli */ object TreadleFirrtlString extends HasShellOptions { val options: Seq[ShellOption[_]] = Seq( @@ -296,7 +296,7 @@ object TreadleFirrtlString extends HasShellOptions { } /** - * Factory for [[FirrtlFileAnnotation]] annotation, this is an alias for Firrtl Cli + * Factory for FirrtlFileAnnotation annotation, this is an alias for Firrtl Cli */ object TreadleFirrtlFile extends HasShellOptions { val options: Seq[ShellOption[_]] = Seq( @@ -315,7 +315,7 @@ case class BlackBoxFactoriesAnnotation(blackBoxFactories: Seq[ScalaBlackBoxFacto /** * Using this annotation allows external users of a TreadleTester to supply their own custom - * [[DataStorePlugin]]s. See that code for examples of use. + * [[treadle.executable.DataStorePlugin]]s. See that code for examples of use. * * @note this annotation cannot be generated from the command line * @param name A unique name for this plugin diff --git a/src/main/scala/treadle/TreadleTester.scala b/src/main/scala/treadle/TreadleTester.scala index f0ec1693..0aa20ffa 100644 --- a/src/main/scala/treadle/TreadleTester.scala +++ b/src/main/scala/treadle/TreadleTester.scala @@ -19,14 +19,13 @@ package treadle import java.io.PrintWriter import java.util.Calendar -import firrtl.{AnnotationSeq, ChirrtlForm, CircuitForm} import firrtl.options.StageOptions import firrtl.options.Viewer.view import firrtl.stage.{FirrtlSourceAnnotation, OutputFileAnnotation} +import firrtl.{AnnotationSeq, ChirrtlForm, CircuitForm} import treadle.chronometry.UTC import treadle.executable._ import treadle.stage.{TreadleCompatibilityPhase, TreadleTesterPhase} -import treadle.vcd.VCD //TODO: Indirect assignments to external modules input is possibly not handled correctly //TODO: Force values should work with multi-slot symbols @@ -117,7 +116,7 @@ class TreadleTester(annotationSeq: AnnotationSeq) { } /** - * Advance time in ticks of the [[UTC]] wallTime, the default is picoseconds, but can be + * Advance time in ticks of the [[treadle.chronometry.UTC]] wallTime, the default is picoseconds, but can be * read by the scaleName of the wallTime. One should probably be advancing by some simple factor * of a clock period. The clockInfoList of the options should define this (could be more than one). * @param interval units are in units of the [[wallTime]] scale. diff --git a/src/main/scala/treadle/executable/DataStorePlugIn.scala b/src/main/scala/treadle/executable/DataStorePlugIn.scala index e55f9a8a..8768cf33 100644 --- a/src/main/scala/treadle/executable/DataStorePlugIn.scala +++ b/src/main/scala/treadle/executable/DataStorePlugIn.scala @@ -16,10 +16,6 @@ limitations under the License. package treadle.executable -import firrtl.annotations.NoTargetAnnotation -import firrtl.options.Unserializable -import treadle.vcd.VCD - import scala.collection.mutable abstract class DataStorePlugin { diff --git a/src/main/scala/treadle/executable/Memory.scala b/src/main/scala/treadle/executable/Memory.scala index 8c79c557..bac7b894 100644 --- a/src/main/scala/treadle/executable/Memory.scala +++ b/src/main/scala/treadle/executable/Memory.scala @@ -17,7 +17,7 @@ limitations under the License. package treadle.executable import treadle._ -import firrtl.{MemKind, WireKind} +import firrtl.{FileUtils, MemKind, WireKind} import firrtl.ir.{ClockType, DefMemory, Info, IntWidth} import RenderHelper.ExpressionHelper import firrtl.annotations.{ComponentName, LoadMemoryAnnotation, MemoryLoadFileType, ModuleName} @@ -433,19 +433,20 @@ object Memory { } } - /** - * Makes a chain of pipeline registers. - * These must be ordered reg0/in, reg0, reg1/in ... regN/in, regN - * This will advance the registers on the specified clock, - * and combinationally pass the register value to the next register's input down the chain - * Data flows from low indexed pipeline elements to high ones - - * @param clock used to create execution based on this trigger. - * @param rootSymbol the head element of the pipeline, this is one of the mem ports - * @param portString name of the writer - * @param pipelineName string representing the name of the root port - * @param latency pipeline latency - */ + + /* + * Makes a chain of pipeline registers. + * These must be ordered reg0/in, reg0, reg1/in ... regN/in, regN + * This will advance the registers on the specified clock, + * and combinationally pass the register value to the next register's input down the chain + * Data flows from low indexed pipeline elements to high ones + + * @param clock used to create execution based on this trigger. + * @param rootSymbol the head element of the pipeline, this is one of the mem ports + * @param portString name of the writer + * @param pipelineName string representing the name of the root port + * @param latency pipeline latency + */ def buildPipelineAssigners(clock: Symbol, rootSymbol: Symbol, portString: String, @@ -601,7 +602,7 @@ class MemoryInitializer(engine: ExecutionEngine) { } private def doInitialize(memorySymbol: Symbol, fileName: String, radix: Int): Unit = { - io.Source.fromFile(fileName).getLines().zipWithIndex.foreach { + FileUtils.getLines(fileName).zipWithIndex.foreach { case (line, lineNumber) if lineNumber < memorySymbol.slots => try { val value = BigInt(line.trim, radix) diff --git a/src/main/scala/treadle/executable/RollBackBuffer.scala b/src/main/scala/treadle/executable/RollBackBuffer.scala index 4667a4c0..da5d4036 100644 --- a/src/main/scala/treadle/executable/RollBackBuffer.scala +++ b/src/main/scala/treadle/executable/RollBackBuffer.scala @@ -16,8 +16,6 @@ limitations under the License. package treadle.executable -import scala.collection.mutable - /** * A RollBackBuffer is the an image of [[DataStore]] at a particular time. * @param dataStore the dataStore to be backed up. diff --git a/src/main/scala/treadle/executable/Snapshotter.scala b/src/main/scala/treadle/executable/Snapshotter.scala index f4d40fb4..4633942f 100644 --- a/src/main/scala/treadle/executable/Snapshotter.scala +++ b/src/main/scala/treadle/executable/Snapshotter.scala @@ -16,14 +16,4 @@ limitations under the License. package treadle.executable -import scala.util.Try - -import org.json4s._ -import org.json4s.native.JsonMethods._ -import org.json4s.native.Serialization -import org.json4s.native.Serialization.{read, write, writePretty} - -import firrtl.ir._ -import firrtl.Utils.error - class Snapshotter {} diff --git a/src/main/scala/treadle/executable/Symbol.scala b/src/main/scala/treadle/executable/Symbol.scala index b5f8d8f8..e4bbde87 100644 --- a/src/main/scala/treadle/executable/Symbol.scala +++ b/src/main/scala/treadle/executable/Symbol.scala @@ -16,10 +16,10 @@ limitations under the License. package treadle.executable -import firrtl.{Kind, WireKind} import firrtl.ir.{Info, IntWidth, NoInfo} +import firrtl.{Kind, WireKind} import treadle._ -import treadle.utils.{BitMasks, BitMasksBigs, BitUtils} +import treadle.utils.{BitMasks, BitMasksBigs} case class Symbol( name: String, diff --git a/src/main/scala/treadle/package.scala b/src/main/scala/treadle/package.scala index e794e284..28214281 100644 --- a/src/main/scala/treadle/package.scala +++ b/src/main/scala/treadle/package.scala @@ -1,9 +1,19 @@ -import java.io.File +/* +Copyright 2020 The Regents of the University of California (Regents) -import treadle.executable.TreadleException -// See LICENSE for license details. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. + */ -//scalastyle:off treadle.object.name package object treadle { import firrtl.ir._ diff --git a/src/main/scala/treadle/repl/ReplOptions.scala b/src/main/scala/treadle/repl/ReplOptions.scala index 553914c1..77813dae 100644 --- a/src/main/scala/treadle/repl/ReplOptions.scala +++ b/src/main/scala/treadle/repl/ReplOptions.scala @@ -20,7 +20,6 @@ import java.io.OutputStream import firrtl.annotations.{Annotation, NoTargetAnnotation} import firrtl.options.{HasShellOptions, ShellOption, Unserializable} -import firrtl.stage.FirrtlFileAnnotation sealed trait ReplOption extends Unserializable { this: Annotation => } diff --git a/src/main/scala/treadle/repl/TreadleReplCli.scala b/src/main/scala/treadle/repl/TreadleReplCli.scala index 73574111..aed7208d 100644 --- a/src/main/scala/treadle/repl/TreadleReplCli.scala +++ b/src/main/scala/treadle/repl/TreadleReplCli.scala @@ -17,7 +17,6 @@ limitations under the License. package treadle.repl import firrtl.options.Shell -import treadle._ trait TreadleReplCli { this: Shell => parser.note("TreadleRepl specific options") diff --git a/src/main/scala/treadle/stage/TreadleStage.scala b/src/main/scala/treadle/stage/TreadleStage.scala index 86a691be..0af97d47 100644 --- a/src/main/scala/treadle/stage/TreadleStage.scala +++ b/src/main/scala/treadle/stage/TreadleStage.scala @@ -16,10 +16,9 @@ limitations under the License. package treadle.stage -import firrtl.{AnnotationSeq, ChirrtlForm, CircuitForm, LowForm} -import firrtl.options.{Phase, Shell, Stage} -import firrtl.stage.FirrtlCli -import treadle.stage.phases.{CreateTester, GetFirrtlAst, PrepareAst, PrepareAstFromLowFIRRTL, SetImplicitOutputInfo} +import firrtl.options.Phase +import firrtl.{AnnotationSeq, ChirrtlForm, CircuitForm} +import treadle.stage.phases._ object TreadleCompatibilityPhase extends Phase { private val chirrtlPhases: Seq[Phase] = Seq( diff --git a/src/main/scala/treadle/stage/phases/PrepareAst.scala b/src/main/scala/treadle/stage/phases/PrepareAst.scala index b1a8e25b..ec8dca1d 100644 --- a/src/main/scala/treadle/stage/phases/PrepareAst.scala +++ b/src/main/scala/treadle/stage/phases/PrepareAst.scala @@ -56,11 +56,9 @@ trait TreadlePhase extends Phase { } } -/** This provides a series of transforms that - * seem to be important to Treadle functionality - * This was based on [[firrtl.LowFirrtlOptimization]] but that - * has includes [[passes.memlib.VerilogMemDelays]] which can cause combinational loops - * for some firrtl files +/** This was a workaround for a problem in firrtl.LowFirrtlOptimization that + * caused problems like combinational loops for treadle's memory simulation. + * This code should not be used going forward */ class TreadleLowFirrtlOptimization extends SeqTransform { def inputForm: CircuitForm = LowForm diff --git a/src/main/scala/treadle/utils/RemoveTempWires.scala b/src/main/scala/treadle/utils/RemoveTempWires.scala index eee4cd1d..566b432f 100644 --- a/src/main/scala/treadle/utils/RemoveTempWires.scala +++ b/src/main/scala/treadle/utils/RemoveTempWires.scala @@ -16,10 +16,8 @@ limitations under the License. package treadle.utils -// See LICENSE for license details. - import firrtl.ir._ -import firrtl.{CircuitForm, CircuitState, LowForm, Parser, Transform, WRef, WSubField, WSubIndex} +import firrtl.{CircuitForm, CircuitState, LowForm, Transform, WRef, WSubField, WSubIndex} import scala.collection.mutable @@ -42,20 +40,20 @@ class RemoveTempWires extends Transform { val c = state.circuit - /** - * removes all references to temp wires in module - * @param module the module to be altered - * @return - */ + /* + * removes all references to temp wires in module + * @param module the module to be altered + * @return + */ def removeTempWiresFromModule(module: Module): Module = { val toRemove = new mutable.HashMap[String, Expression]() - /** - * Saves reference to the expression associated - * with a temp wire associated with a Node statement - * @param s statement to be checked - */ + /* + * Saves reference to the expression associated + * with a temp wire associated with a Node statement + * @param s statement to be checked + */ def collectTempExpressions(s: Statement): Unit = s match { case block: Block => block.stmts.foreach { substatement => @@ -74,12 +72,12 @@ class RemoveTempWires extends Transform { case _ => //do nothing } - /** - * recursively find any references to temp wires in the expression and replace the - * references with the associated expression - * @param e expression to be altered - * @return - */ + /* + * recursively find any references to temp wires in the expression and replace the + * references with the associated expression + * @param e expression to be altered + * @return + */ def removeGen(e: Expression): Expression = { e match { case wire: WRef => @@ -104,11 +102,11 @@ class RemoveTempWires extends Transform { } } - /** - * Removes node definition statements for temp wires - * @param s statement to be altered - * @return - */ + /* + * Removes node definition statements for temp wires + * @param s statement to be altered + * @return + */ def removeGenStatement(s: Statement): Option[Statement] = { s match { case block: Block => From 3a1dabeb6141736483614eb1a318b43bff8bae69 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 5 Mar 2020 13:42:59 -0800 Subject: [PATCH 2/4] Return to using LowFirrtlOptimization This will invoke VerilogMemDelays stuff that will build register piplelines, instead of having treadle do it. This PR changes Memory.scala, to set all latencies to zero to rely on what was assembled by the VerilogMemDelays. I'm keeping all the machinery to build these pipelines in Memory.scala since there is a // Todo on VerilogMemDelays in LowFirrtlOptimization to move it to verilog emitter. This includes a fix for registers created by Memory to be RegKind instead of WireKind --- .../scala/treadle/executable/Memory.scala | 67 ++++++++++++----- .../treadle/executable/SymbolTable.scala | 2 +- .../treadle/stage/phases/PrepareAst.scala | 3 +- src/test/scala/treadle/ClockSpec.scala | 2 +- src/test/scala/treadle/MemoryUsageSpec.scala | 75 ++++++++++++++++++- 5 files changed, 127 insertions(+), 22 deletions(-) diff --git a/src/main/scala/treadle/executable/Memory.scala b/src/main/scala/treadle/executable/Memory.scala index 8c79c557..be3f5d45 100644 --- a/src/main/scala/treadle/executable/Memory.scala +++ b/src/main/scala/treadle/executable/Memory.scala @@ -17,13 +17,35 @@ limitations under the License. package treadle.executable import treadle._ -import firrtl.{MemKind, WireKind} -import firrtl.ir.{ClockType, DefMemory, Info, IntWidth} +import firrtl.{FileUtils, MemKind, RegKind, WireKind} +import firrtl.ir.{ClockType, DefMemory, Info, IntWidth, ReadUnderWrite} import RenderHelper.ExpressionHelper import firrtl.annotations.{ComponentName, LoadMemoryAnnotation, MemoryLoadFileType, ModuleName} import scala.collection.mutable +//////////////////////////////////////////////////////////////////////////////// +// IMPORTANT NOTE +// The primary methods here have been effectively disabled by using +// val memory = defMemory.copy(readLatency = 0, writeLatency = 0, +// readUnderWrite = ReadUnderWrite.Undefined) +// appearing at the beginning. This is because the functionality contained here, +// i.e. adding read and write pipeline registers is now being taken care of in the +// VerilogMemDelays transform in the LowFirrtlOptimization. This may or may not +// be the long term solution. So the code here will remain here but disabled for now. +// Though it is still used to do the trivial case of hooking up memory ports to the +// wires +//////////////////////////////////////////////////////////////////////////////// + +/** Provides three different aspects of the code necessary to create read and write + * register pipelines. The three cases are: + * - Adding the symbols for the registers to be created + * - Adding the rendering code necessary to Expression views of the pipelines + * - Actually adding the pipeline registers. + * + * NOTE: See IMPORTANT NOTE above. + * + */ object Memory { //scalastyle:off method.length /** @@ -31,17 +53,19 @@ object Memory { * Pipelines are constructed as registers with a regular name and * a /in name. Data travels up-index through a pipeline for both * read and write pipelines. - * @param memory the specified memory + * @param defMemory the specified memory * @param expandedName the full name of the memory * @param sensitivityGraphBuilder external graph of dependencies * @return */ def buildSymbols( - memory: DefMemory, + defMemory: DefMemory, expandedName: String, sensitivityGraphBuilder: SensitivityGraphBuilder, registerNames: mutable.HashSet[String] ): Seq[Symbol] = { + val memory = defMemory.copy(readLatency = 0, writeLatency = 0, readUnderWrite = ReadUnderWrite.Undefined) + if (memory.depth >= BigInt(Int.MaxValue)) { throw TreadleException(s"Memory $expandedName size ${memory.depth} is too large for treadle") } @@ -53,9 +77,11 @@ object Memory { val lastValueSymbols = new mutable.ArrayBuffer[Symbol]() + val effectiveReadLatency = memory.readLatency + (if (memory.readUnderWrite == ReadUnderWrite.New) 1 else 0) + def buildRegisterTriple(baseName: String, index: Int, dataType: firrtl.ir.Type): Seq[Symbol] = { - val register = Symbol(s"$baseName$index", dataType, WireKind) + val register = Symbol(s"$baseName$index", dataType, RegKind) registerNames += register.name val registerIn = SymbolTable.makeRegisterInputSymbol(register) val lastClockValue = SymbolTable.makeLastValueSymbol(register) @@ -104,10 +130,10 @@ object Memory { sensitivityGraphBuilder.addSensitivity(clk, data) - val pipelineRaddrSymbols = (0 until memory.readLatency).flatMap { n => + val pipelineRaddrSymbols = (0 until effectiveReadLatency).flatMap { n => buildRegisterTriple(s"$expandedName.$readerString.pipeline_raddr_", n, addrType) } - val pipelineEnableSymbols = (0 until memory.readLatency).flatMap { n => + val pipelineEnableSymbols = (0 until effectiveReadLatency).flatMap { n => buildRegisterTriple(s"$expandedName.$readerString.pipeline_ren_", n, booleanType) } @@ -175,12 +201,12 @@ object Memory { sensitivityGraphBuilder.addSensitivity(mode, valid) sensitivityGraphBuilder.addSensitivity(mask, valid) - val pipelineReadDataSymbols = (0 until memory.readLatency).flatMap { n => + val pipelineReadDataSymbols = (0 until effectiveReadLatency).flatMap { n => buildRegisterTriple(s"$expandedName.$readWriterString.pipeline_raddr_", n, addrType) } buildPipelineDependencies(addr, pipelineReadDataSymbols, Some(rdata), clockSymbol = Some(clk)) - val pipelineReadEnableSymbols = (0 until memory.readLatency).flatMap { n => + val pipelineReadEnableSymbols = (0 until effectiveReadLatency).flatMap { n => buildRegisterTriple(s"$expandedName.$readWriterString.pipeline_ren_", n, booleanType) } buildPipelineDependencies(addr, pipelineReadEnableSymbols, Some(rdata), clockSymbol = Some(clk)) @@ -215,18 +241,19 @@ object Memory { /** * Construct views for all the memory elements - * @param memory current memory + * @param defMemory current memory * @param expandedName full path name * @param scheduler handle to execution components * @param expressionViews where to store the generated views */ def buildMemoryExpressions( - memory: DefMemory, + defMemory: DefMemory, expandedName: String, scheduler: Scheduler, // compiler : ExpressionCompiler, expressionViews: mutable.HashMap[Symbol, ExpressionView] ): Unit = { + val memory = defMemory.copy(readLatency = 0, writeLatency = 0, readUnderWrite = ReadUnderWrite.Undefined) val symbolTable = scheduler.symbolTable val memorySymbol = symbolTable(expandedName) @@ -269,7 +296,9 @@ object Memory { enable: Symbol ): Symbol = { - val pipelineReadSymbols = buildPipeLine(portName, pipelineName, memory.readLatency) + val effectiveReadLatency = memory.readLatency + (if (memory.readUnderWrite == ReadUnderWrite.New) 1 else 0) + + val pipelineReadSymbols = buildPipeLine(portName, pipelineName, effectiveReadLatency) val chain = Seq(addr) ++ pipelineReadSymbols // This produces triggered: reg0 <= reg0/in, reg1 <= reg1/in etc. @@ -391,17 +420,19 @@ object Memory { /** * Construct the machinery to move data into and out of the memory stack - * @param memory current memory + * @param defMemory current memory * @param expandedName full path name * @param scheduler handle to execution components * @param compiler needed for assigner generation */ def buildMemoryInternals( - memory: DefMemory, + defMemory: DefMemory, expandedName: String, scheduler: Scheduler, compiler: ExpressionCompiler ): Unit = { + val memory = defMemory.copy(readLatency = 0, writeLatency = 0, readUnderWrite = ReadUnderWrite.Undefined) + val symbolTable = scheduler.symbolTable val memorySymbol = symbolTable(expandedName) val dataStore = compiler.dataStore @@ -482,8 +513,10 @@ object Memory { val addr = symbolTable(s"$name.addr") val data = symbolTable(s"$name.data") - val endOfAddrPipeline = buildPipelineAssigners(clock, addr, name, "raddr", memory.readLatency, memory.info) - val endOfEnablePipeline = buildPipelineAssigners(clock, enable, name, "ren", memory.readLatency, memory.info) + val effectiveReadLatency = memory.readLatency + (if (memory.readUnderWrite == ReadUnderWrite.New) 1 else 0) + + val endOfAddrPipeline = buildPipelineAssigners(clock, addr, name, "raddr", effectiveReadLatency, memory.info) + val endOfEnablePipeline = buildPipelineAssigners(clock, enable, name, "ren", effectiveReadLatency, memory.info) compiler.makeAssigner( data, @@ -601,7 +634,7 @@ class MemoryInitializer(engine: ExecutionEngine) { } private def doInitialize(memorySymbol: Symbol, fileName: String, radix: Int): Unit = { - io.Source.fromFile(fileName).getLines().zipWithIndex.foreach { + FileUtils.getLines(fileName).zipWithIndex.foreach { case (line, lineNumber) if lineNumber < memorySymbol.slots => try { val value = BigInt(line.trim, radix) diff --git a/src/main/scala/treadle/executable/SymbolTable.scala b/src/main/scala/treadle/executable/SymbolTable.scala index 59d4c220..93f02480 100644 --- a/src/main/scala/treadle/executable/SymbolTable.scala +++ b/src/main/scala/treadle/executable/SymbolTable.scala @@ -160,7 +160,7 @@ object SymbolTable extends LazyLogging { def makeRegisterInputName(name: String): String = name + RegisterInputSuffix def makeRegisterInputName(symbol: Symbol): String = symbol.name + RegisterInputSuffix def makeRegisterInputSymbol(symbol: Symbol): Symbol = { - Symbol(makeRegisterInputName(symbol), symbol.firrtlType, WireKind, info = symbol.info) + Symbol(makeRegisterInputName(symbol), symbol.firrtlType, RegKind, info = symbol.info) } def makeLastValueName(name: String): String = name + LastValueSuffix diff --git a/src/main/scala/treadle/stage/phases/PrepareAst.scala b/src/main/scala/treadle/stage/phases/PrepareAst.scala index b1a8e25b..8c33b2c8 100644 --- a/src/main/scala/treadle/stage/phases/PrepareAst.scala +++ b/src/main/scala/treadle/stage/phases/PrepareAst.scala @@ -29,6 +29,7 @@ import firrtl.{ CircuitForm, CircuitState, HighForm, + LowFirrtlOptimization, LowForm, SeqTransform, Transform, @@ -134,7 +135,7 @@ object PrepareAst extends TreadlePhase { Seq( new ChirrtlToLow, new HighToLow, - new TreadleLowFirrtlOptimization, + new LowFirrtlOptimization, new BlackBoxSourceHelper, new FixupOps, AugmentPrintf diff --git a/src/test/scala/treadle/ClockSpec.scala b/src/test/scala/treadle/ClockSpec.scala index d8d35c0b..447a7b70 100644 --- a/src/test/scala/treadle/ClockSpec.scala +++ b/src/test/scala/treadle/ClockSpec.scala @@ -120,7 +120,7 @@ class ClockSpec extends FreeSpec with Matchers { println(s"memory($i) = ${tester.peekMemory("m", i)}") } // read phase - tester.poke("write_en", 1) + tester.poke("write_en", 0) for (i <- 0 until 8) { tester.poke("addr", i) tester.expect("out1", i * 10 + i) diff --git a/src/test/scala/treadle/MemoryUsageSpec.scala b/src/test/scala/treadle/MemoryUsageSpec.scala index 56619839..9e26cae4 100644 --- a/src/test/scala/treadle/MemoryUsageSpec.scala +++ b/src/test/scala/treadle/MemoryUsageSpec.scala @@ -22,6 +22,7 @@ import firrtl.FileUtils import firrtl.annotations.{CircuitName, ComponentName, LoadMemoryAnnotation, ModuleName} import firrtl.stage.FirrtlSourceAnnotation import org.scalatest.{FreeSpec, Matchers} +import treadle.executable.StopException /** * Created by chick on 4/30/16. @@ -307,7 +308,7 @@ class MemoryUsageSpec extends FreeSpec with Matchers { tester.poke("in1", 11) tester.poke("addr", 3) tester.poke("write_en", 1) - tester.expectMemory("m", 3, 0) + tester.expectMemory("m", 3, 11) tester.step() @@ -450,7 +451,7 @@ class MemoryUsageSpec extends FreeSpec with Matchers { tester.finish } - val simpleMem = + private val simpleMem = s""" |circuit a : | module a : @@ -520,4 +521,74 @@ class MemoryUsageSpec extends FreeSpec with Matchers { tester.step() tester.peek("m.r.en") should be(0) } + + "SyncReadMem write collision behaviors should work" in { + val input = + """ + |;buildInfoPackage: chisel3, version: 3.3-SNAPSHOT, scalaVersion: 2.12.10, sbtVersion: 1.3.8 + |circuit SyncReadMemWriteCollisionTester : + | module SyncReadMemWriteCollisionTester : + | input clock : Clock + | input reset : UInt<1> + | output io : {} + | + | reg cnt : UInt<3>, clock with : (reset => (reset, UInt<3>("h00"))) @[Counter.scala 29:33] + | when UInt<1>("h01") : @[Counter.scala 71:17] + | node _T = eq(cnt, UInt<3>("h04")) @[Counter.scala 37:24] + | node _T_1 = add(cnt, UInt<1>("h01")) @[Counter.scala 38:22] + | node _T_2 = tail(_T_1, 1) @[Counter.scala 38:22] + | cnt <= _T_2 @[Counter.scala 38:13] + | when _T : @[Counter.scala 40:21] + | cnt <= UInt<1>("h00") @[Counter.scala 40:29] + | skip @[Counter.scala 40:21] + | skip @[Counter.scala 71:17] + | node _T_3 = and(UInt<1>("h01"), _T) @[Counter.scala 72:20] + | smem m0 : UInt<2>[2], new @[Mem.scala 40:23] + | node _T_4 = bits(cnt, 0, 0) @[Mem.scala 41:20] + | read mport rd0 = m0[_T_4], clock @[Mem.scala 41:20] + | node _T_5 = bits(cnt, 0, 0) + | write mport _T_6 = m0[_T_5], clock + | _T_6 <= cnt + | smem m1 : UInt<2>[2], old @[Mem.scala 45:23] + | node _T_7 = bits(cnt, 0, 0) @[Mem.scala 46:20] + | read mport rd1 = m1[_T_7], clock @[Mem.scala 46:20] + | node _T_8 = bits(cnt, 0, 0) + | write mport _T_9 = m1[_T_8], clock + | _T_9 <= cnt + | node _T_10 = eq(cnt, UInt<2>("h03")) @[Mem.scala 50:13] + | when _T_10 : @[Mem.scala 50:22] + | node _T_11 = eq(rd0, UInt<2>("h02")) @[Mem.scala 51:16] + | node _T_12 = bits(reset, 0, 0) @[Mem.scala 51:11] + | node _T_13 = or(_T_11, _T_12) @[Mem.scala 51:11] + | node _T_14 = eq(_T_13, UInt<1>("h00")) @[Mem.scala 51:11] + | when _T_14 : @[Mem.scala 51:11] + | printf(clock, UInt<1>(1), "Assertion failed\n at Mem.scala:51 assert(rd0 === 2.U)\n") @[Mem.scala 51:11] + | stop(clock, UInt<1>(1), 1) @[Mem.scala 51:11] + | skip @[Mem.scala 51:11] + | node _T_15 = eq(rd1, UInt<1>("h00")) @[Mem.scala 52:16] + | node _T_16 = bits(reset, 0, 0) @[Mem.scala 52:11] + | node _T_17 = or(_T_15, _T_16) @[Mem.scala 52:11] + | node _T_18 = eq(_T_17, UInt<1>("h00")) @[Mem.scala 52:11] + | when _T_18 : @[Mem.scala 52:11] + | printf(clock, UInt<1>(1), "Assertion failed\n at Mem.scala:52 assert(rd1 === 0.U)\n") @[Mem.scala 52:11] + | stop(clock, UInt<1>(1), 1) @[Mem.scala 52:11] + | skip @[Mem.scala 52:11] + | skip @[Mem.scala 50:22] + | node _T_19 = eq(cnt, UInt<3>("h04")) @[Mem.scala 55:13] + | when _T_19 : @[Mem.scala 55:22] + | node _T_20 = bits(reset, 0, 0) @[Mem.scala 56:9] + | node _T_21 = eq(_T_20, UInt<1>("h00")) @[Mem.scala 56:9] + | when _T_21 : @[Mem.scala 56:9] + | stop(clock, UInt<1>(1), 0) @[Mem.scala 56:9] + | skip @[Mem.scala 56:9] + | skip @[Mem.scala 55:22] + | + |""".stripMargin + + val tester = TreadleTester(Seq(FirrtlSourceAnnotation(input), WriteVcdAnnotation, ShowFirrtlAtLoadAnnotation)) + + intercept[StopException] { + tester.step(100) + } + } } From ac8fe2102f98e41578315df8981b93c8894ee706 Mon Sep 17 00:00:00 2001 From: Jim Lawson Date: Fri, 24 Apr 2020 17:17:23 -0700 Subject: [PATCH 3/4] Address Scala 2.11 issue; add missing import. --- src/main/scala/treadle/executable/Memory.scala | 4 ++-- src/test/scala/treadle/MemoryUsageSpec.scala | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/scala/treadle/executable/Memory.scala b/src/main/scala/treadle/executable/Memory.scala index be3f5d45..24cb4881 100644 --- a/src/main/scala/treadle/executable/Memory.scala +++ b/src/main/scala/treadle/executable/Memory.scala @@ -18,7 +18,7 @@ package treadle.executable import treadle._ import firrtl.{FileUtils, MemKind, RegKind, WireKind} -import firrtl.ir.{ClockType, DefMemory, Info, IntWidth, ReadUnderWrite} +import firrtl.ir.{ClockType, DefMemory, Info, IntWidth, ReadUnderWrite, Type} import RenderHelper.ExpressionHelper import firrtl.annotations.{ComponentName, LoadMemoryAnnotation, MemoryLoadFileType, ModuleName} @@ -72,7 +72,7 @@ object Memory { val memorySymbol = Symbol(expandedName, memory.dataType, MemKind, memory.depth.toInt) val addrWidth = IntWidth(requiredBitsForUInt(memory.depth - 1)) val addrType = firrtl.ir.UIntType(addrWidth) - val dataType = memory.dataType + val dataType: Type = memory.dataType val booleanType = firrtl.ir.UIntType(IntWidth(1)) val lastValueSymbols = new mutable.ArrayBuffer[Symbol]() diff --git a/src/test/scala/treadle/MemoryUsageSpec.scala b/src/test/scala/treadle/MemoryUsageSpec.scala index 2792e469..8b9d8512 100644 --- a/src/test/scala/treadle/MemoryUsageSpec.scala +++ b/src/test/scala/treadle/MemoryUsageSpec.scala @@ -21,6 +21,7 @@ import java.io.{File, PrintWriter} import firrtl.FileUtils import firrtl.annotations.{CircuitName, ComponentName, LoadMemoryAnnotation, ModuleName} import firrtl.stage.FirrtlSourceAnnotation +import treadle.executable.StopException import org.scalatest.freespec.AnyFreeSpec import org.scalatest.matchers.should.Matchers From 3495ed0d3aed80aa37a7dd9ec7e047d2b84dfaf0 Mon Sep 17 00:00:00 2001 From: Chick Markley Date: Tue, 28 Apr 2020 09:53:11 -0700 Subject: [PATCH 4/4] Add github action/workflow testing (#192) * Blatant copy and edit from firrtl * Blatant copy and edit from firrtl-interpreter * Looks like we needed a few more files here * Fixed badge in README.md --- ...tractBuildDependenciesFromPullRequest.gawk | 66 +++++++++++ .github/scripts/generateGitClones.gawk | 29 +++++ .github/scripts/generateVersionOverrides.gawk | 21 ++++ .github/workflows/test.yml | 106 ++++++++++++++++++ README.md | 5 + 5 files changed, 227 insertions(+) create mode 100644 .github/scripts/ExtractBuildDependenciesFromPullRequest.gawk create mode 100644 .github/scripts/generateGitClones.gawk create mode 100644 .github/scripts/generateVersionOverrides.gawk create mode 100644 .github/workflows/test.yml diff --git a/.github/scripts/ExtractBuildDependenciesFromPullRequest.gawk b/.github/scripts/ExtractBuildDependenciesFromPullRequest.gawk new file mode 100644 index 00000000..a7306705 --- /dev/null +++ b/.github/scripts/ExtractBuildDependenciesFromPullRequest.gawk @@ -0,0 +1,66 @@ +/^[[:space:]]*"body": / { + # Convert character sequences to character constants + gsub(/\\r/, "") + gsub(/\\n/, "\n") + # Remove trailing punctuation + gsub(/\n*",$/, "") + # Look for the build dependency tag + i=split($0, a, /### Build Dependencies/) + if ( i != 2 ) { + print "no build dependencies" + exit 0 + } + # Throw away everything up to the build dependency tag + $0=a[2] + # Grab the specific build dependencies + i=split($0,a,/\nBuild with: /) + delete a[1] + # Format the individual dependencies + errLines = 0 + depLines = 0 + for (i in a) { + # Remove trailing text + gsub(/\n.*$/, "", a[i]) + n=split(a[i], d, /\s+/) + switch (d[1]) { + case /maven-version/ : + if (n != 3) { + err[errLines++]="unrecognized maven-version stanza: " a[i] "(" n ")" + } + break + + case /git-clone/ : + if (n != 4) { + err[errLines++]="unrecognized git-clone stanza: " a[i] "(" n ")" + } + break + + default: + err[errLines++]="unrecognized build type: " d[1] " - " a[i] + break + + } + # If we don't have any errors, format this dependency line and add it + # to the list of dependency lines. + if (errLines == 0) { + sep="" + line="" + for (n in d) { + line=line sep d[n] + sep=" " + } + dep[depLines++]=line + } + } + # Print either errors (to stderr) or the dependency lines (to stdout) + if (errLines == 0) { + for (n in dep) { + print dep[n] + } + } else { + for (n in err) { + print err[n] > "/dev/stderr" + } + exit 1 + } +} diff --git a/.github/scripts/generateGitClones.gawk b/.github/scripts/generateGitClones.gawk new file mode 100644 index 00000000..301bf604 --- /dev/null +++ b/.github/scripts/generateGitClones.gawk @@ -0,0 +1,29 @@ +# Generate commands to: +# - clone, build, and publish dependencies, (on stdout) +# - fetch corresponding shas ( on fd 3) +# Specify "-v verbose=1" on the command line for verbose output (to stderr) +/#/ { + # Strip comments + gsub(/#.*$/,"") +} +/git-clone/ { + project=$2 + repo=$3 + tag=$4 + if (verbose) { + print "writing clone and build commands to stdout." > "/dev/stderr" + } + # Generate commands to do a shallow fetch of all branches, checkout the desired ref/sha, and build and publish the jars. + printf "git clone --no-single-branch --no-checkout --depth 5 %s %s && (cd %s && git checkout %s && sbt +publishLocal)\n",repo,project,project,tag + # If the tag looks like a SHA, assume it is and generate a command to just echo it, + # otherwise, generate a string to fetch the sha from the remote. + # In either case, write the output to fd 3. + if (verbose) { + print "writing sha generation commands to fd 3." > "/dev/stderr" + } + if ( tag ~ /^[[:xdigit:]]+$/ ) { + printf "echo %s\n", tag > "/dev/fd/3" + } else { + printf "git ls-remote --tags --heads %s %s", repo, tag > "/dev/fd/3" + } +} diff --git a/.github/scripts/generateVersionOverrides.gawk b/.github/scripts/generateVersionOverrides.gawk new file mode 100644 index 00000000..3f8f0e61 --- /dev/null +++ b/.github/scripts/generateVersionOverrides.gawk @@ -0,0 +1,21 @@ +# Generate suitable definitions for sbt to override default dependency +# specifications in the associated build.sbt, assuming the latter has +# been structured to support this. It should contain something like: +# +# // Provide a managed dependency on X if -DXVersion="" is supplied on the command line. +# val defaultVersions = Seq( +# "chisel3" -> "3.3-SNAPSHOT", +# "treadle" -> "1.2-SNAPSHOT" +# ) +# +# libraryDependencies ++= defaultVersions.map { case (dep, ver) => +# "edu.berkeley.cs" %% dep % sys.props.getOrElse(dep + "Version", ver) } + +/#/ { + # Strip comments + gsub(/#.*$/,"") +} +/maven-version/ { + # Print a series of "-DfooVersion=xxx" to override the default chisel versions in build.sbt + printf "%s-D%sVersion=%s", sep, $2, $3; sep = " " +} diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 00000000..243876ec --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,106 @@ +name: Test + +on: [pull_request] + +jobs: + build: + name: ci + runs-on: ubuntu-latest + container: + image: ucbbar/chisel3-tools + options: --user github --entrypoint /bin/bash + env: + CHISEL3_REF: master + FIRRTL_REF: master + FIRRTL_INTERPRETER_REF: master + TREADLE_REF: master + CONTAINER_HOME: /home/github + + steps: + - name: id + id: id + run: | + uid=$(id -u) + echo ::set-env name=CONTAINER_HOME::$(if [ "$uid" = "0" ]; then echo "/root"; else echo "/home/github"; fi) + printenv + whoami + git --version + pwd + # Currently, sbt seems to either ignore (or doesn't see) $HOME inside the container, + # and insists on using /home/ (i.e., /home/github). + # Set up symbolic links so /home/gitsub/.{sbt,cache} (inside the container) are links to + # the equivalent directories in $HOME (i.e, /github/home) + - name: link-caches + id: link-caches + run: | + echo "Link $CONTAINER_HOME caches to $HOME" + echo mkdir -p $HOME/.cache $HOME/.ivy2 $HOME/.sbt + mkdir -p $HOME/.cache $HOME/.ivy2 $HOME/.sbt + echo ln -s $HOME/.cache $HOME/.ivy2 $HOME/.sbt $CONTAINER_HOME + ln -s $HOME/.cache $HOME/.ivy2 $HOME/.sbt $CONTAINER_HOME + echo ls -la $HOME . $CONTAINER_HOME + ls -la $HOME . $CONTAINER_HOME + - name: checkout + uses: actions/checkout@v2 + with: + path: repo + - name: cache-sbt + uses: actions/cache@v1 + env: + cache-name: cache-sbt + with: + path: ~/.sbt + key: build-${{ env.cache-name }}-v1 + restore-keys: | + build-${{ env.cache-name }}- + - name: cache-coursier + uses: actions/cache@v1 + env: + cache-name: cache-coursier + with: + path: ~/.cache + key: build-${{ env.cache-name }}-v1 + restore-keys: | + build-${{ env.cache-name }}- + - name: list + id: list + run: | + echo ls -la . repo ~/.sbt ~/.cache /home/github /home/github/.??* + ls -la . repo ~/.sbt ~/.cache /home/github /home/github/.??* + - name: env + id: env + run: | + echo "cat $GITHUB_EVENT_PATH" + cat $GITHUB_EVENT_PATH + - name: gawk + id: gawk + run: | + gawk -f repo/.github/scripts/ExtractBuildDependenciesFromPullRequest.gawk $GITHUB_EVENT_PATH > deps + ls -l deps + cat deps + gawk -f repo/.github/scripts/generateGitClones.gawk deps > clones.sh 3> shas.sh + cat shas.sh + bash shas.sh > shas + - name: cache-dependencies + id: cache-dependencies + uses: actions/cache@v1 + env: + cache-name: cache-dependencies + with: + path: ~/.ivy2/local + key: build-${{ env.cache-name }}-v1-${{ hashFiles('shas') }} + - name: clone-deps + id: clone-deps + run: | + cat clones.sh + bash clones.sh + if: steps.cache-dependencies.outputs.cache-hit != 'true' + - name: version-deps + id: version-deps + run: | + versionDeps=$(gawk -f repo/.github/scripts/generateVersionOverrides.gawk deps) + echo "versonDeps: $versionDeps" + - name: test + id: test + run: cat /dev/null | sbt $versionDeps +test + working-directory: ./repo diff --git a/README.md b/README.md index 4c54208d..c7955b18 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,11 @@ Treadle -- A Chisel/Firrtl Execution Engine ================== +--- + +[![Join the chat at https://gitter.im/freechipsproject/firrtl](https://badges.gitter.im/freechipsproject/firrtl.svg)](https://gitter.im/freechipsproject/firrtl?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) +![Test](https://github.com/freechipsproject/treadle/workflows/Test/badge.svg) + **Treadle** is an experimental circuit simulator that executes low Firrtl IR. It is based on earlier work on the [firrtl_interpreter](https://github.com/freechipsproject/firrtl-interpreter) It will be one of the standard back-ends available as part of