From 32430bcfc49b0293748ba3f6e5000a45a3dcce92 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 1 Apr 2022 17:24:56 +0000 Subject: Prevent FIRRTL bulk connects on BlackBox Bundles. (#2468) (#2469) (cherry picked from commit 4da1e89f3a0b79adcb39ea5defb393ed6c00fa2f) Co-authored-by: fzi-hielscher <47524191+fzi-hielscher@users.noreply.github.com>--- src/test/scala/chiselTests/BulkConnectSpec.scala | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/BulkConnectSpec.scala b/src/test/scala/chiselTests/BulkConnectSpec.scala index 463122bd..281890d4 100644 --- a/src/test/scala/chiselTests/BulkConnectSpec.scala +++ b/src/test/scala/chiselTests/BulkConnectSpec.scala @@ -94,6 +94,26 @@ class BulkConnectSpec extends ChiselPropSpec { chirrtl should include("deq <= enq") } + property("Chisel connects should not emit a FIRRTL bulk connect for BlackBox IO Bundles") { + class MyBundle extends Bundle { + val O: Bool = Output(Bool()) + val I: Bool = Input(Bool()) + } + + val chirrtl = ChiselStage.emitChirrtl(new Module { + val io: MyBundle = IO(Flipped(new MyBundle)) + + val bb = Module(new BlackBox { + val io: MyBundle = IO(Flipped(new MyBundle)) + }) + + io <> bb.io + }) + // There won't be a bb.io Bundle in FIRRTL, so connections have to be done element-wise + chirrtl should include("bb.O <= io.O") + chirrtl should include("io.I <= bb.I") + } + property("MonoConnect should bulk connect undirectioned internal wires") { val chirrtl = ChiselStage.emitChirrtl(new Module { val io = IO(new Bundle {}) -- cgit v1.2.3 From 898142ba05b04fb1602b249fd1ae81baa3f47f89 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 12 Apr 2022 00:09:55 +0000 Subject: Enhance views to [sometimes] support dynamic indexing and implement FlatIO (backport #2476) (#2479) * Capture 1:1 mappings of Aggregates inside of views This is implemented by including any corresponding Aggregates from the DataView.mapping in the AggregateViewBinding.childMap (which is now of type Map[Data, Data]). This enables dynamically indexing Vecs that are themselves elements of larger Aggregates in views when the corresponding element of the view is a Vec of the same type. It also increases the number of cases where a single Target can represent part of a view. (cherry picked from commit 1f6b1ca14ccf86918065073c3f6f3626dd83a68e) * Add FlatIO API for creating ports from Bundles without a prefix (cherry picked from commit 772a3a1fe3b9372b7c2d7cd2d424b2adcd633cdb) * [docs] Add FlatIO to the general cookbook (cherry picked from commit b4159641350f238f0f899b69954142ce8ee11544) Co-authored-by: Jack Koenig --- .../scala/chiselTests/experimental/DataView.scala | 30 +++++++++++++ .../experimental/DataViewTargetSpec.scala | 4 +- .../chiselTests/experimental/FlatIOSpec.scala | 51 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 src/test/scala/chiselTests/experimental/FlatIOSpec.scala (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/experimental/DataView.scala b/src/test/scala/chiselTests/experimental/DataView.scala index 0285a524..e7caacfd 100644 --- a/src/test/scala/chiselTests/experimental/DataView.scala +++ b/src/test/scala/chiselTests/experimental/DataView.scala @@ -332,6 +332,36 @@ class DataViewSpec extends ChiselFlatSpec { chirrtl should include("dataOut <= vec[addr]") } + it should "support dynamic indexing for Vecs that correspond 1:1 in a view" in { + class MyBundle extends Bundle { + val foo = Vec(4, UInt(8.W)) + val bar = UInt(2.W) + } + implicit val myView = DataView[(Vec[UInt], UInt), MyBundle]( + _ => new MyBundle, + _._1 -> _.foo, + _._2 -> _.bar + ) + class MyModule extends Module { + val dataIn = IO(Input(UInt(8.W))) + val addr = IO(Input(UInt(2.W))) + val dataOut = IO(Output(UInt(8.W))) + + val vec = RegInit(0.U.asTypeOf(Vec(4, UInt(8.W)))) + val addrReg = Reg(UInt(2.W)) + val view = (vec, addrReg).viewAs[MyBundle] + // Dynamic indexing is more of a "generator" in Chisel3 than an individual node + // This style is not recommended, this is just testing the behavior + val selected = view.foo(view.bar) + view.bar := addr + selected := dataIn + dataOut := selected + } + val chirrtl = ChiselStage.emitChirrtl(new MyModule) + chirrtl should include("vec[addrReg] <= dataIn") + chirrtl should include("dataOut <= vec[addrReg]") + } + it should "error if you try to dynamically index a Vec view that does not correspond to a Vec target" in { class MyModule extends Module { val inA, inB = IO(Input(UInt(8.W))) diff --git a/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala b/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala index da27c9c8..ddeeab6e 100644 --- a/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala +++ b/src/test/scala/chiselTests/experimental/DataViewTargetSpec.scala @@ -125,9 +125,7 @@ class DataViewTargetSpec extends ChiselFlatSpec { val pairs = annos.collect { case DummyAnno(t, idx) => (idx, t.toString) }.sortBy(_._1) val expected = Seq( 0 -> "~MyParent|MyChild>out.foo", - // The child of the view that was itself an Aggregate got split because 1:1 is lacking here - 1 -> "~MyParent|MyChild>out.foo[0]", - 1 -> "~MyParent|MyChild>out.foo[1]", + 1 -> "~MyParent|MyChild>out.foo", 2 -> "~MyParent|MyParent/inst:MyChild>out.foo", 3 -> "~MyParent|MyParent/inst:MyChild>out" ) diff --git a/src/test/scala/chiselTests/experimental/FlatIOSpec.scala b/src/test/scala/chiselTests/experimental/FlatIOSpec.scala new file mode 100644 index 00000000..dfce447f --- /dev/null +++ b/src/test/scala/chiselTests/experimental/FlatIOSpec.scala @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests.experimental + +import chisel3._ +import chisel3.util.Valid +import chisel3.stage.ChiselStage.emitChirrtl +import chisel3.experimental.FlatIO +import chiselTests.ChiselFlatSpec + +class FlatIOSpec extends ChiselFlatSpec { + behavior.of("FlatIO") + + it should "create ports without a prefix" in { + class MyModule extends RawModule { + val io = FlatIO(new Bundle { + val in = Input(UInt(8.W)) + val out = Output(UInt(8.W)) + }) + io.out := io.in + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("input in : UInt<8>") + chirrtl should include("output out : UInt<8>") + chirrtl should include("out <= in") + } + + it should "support bulk connections between FlatIOs and regular IOs" in { + class MyModule extends RawModule { + val in = FlatIO(Input(Valid(UInt(8.W)))) + val out = IO(Output(Valid(UInt(8.W)))) + out := in + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("out.bits <= bits") + chirrtl should include("out.valid <= valid") + } + + it should "dynamically indexing Vecs inside of FlatIOs" in { + class MyModule extends RawModule { + val io = FlatIO(new Bundle { + val addr = Input(UInt(2.W)) + val in = Input(Vec(4, UInt(8.W))) + val out = Output(Vec(4, UInt(8.W))) + }) + io.out(io.addr) := io.in(io.addr) + } + val chirrtl = emitChirrtl(new MyModule) + chirrtl should include("out[addr] <= in[addr]") + } +} -- cgit v1.2.3 From b187a0ce5269e735e7bd75620b4fd33f7ed27af5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 15 Apr 2022 19:45:25 +0000 Subject: Enable Clock Invalidation (#2485) (#2487) Loosen restrictions on clocks to enable them to be connected to DontCare, i.e., be invalidated. Co-authored-by: Jack Koenig Signed-off-by: Schuyler Eldridge Co-authored-by: Jack Koenig (cherry picked from commit 5d8a0c8e406376f7ceda91273fb0fa7a646865aa) Co-authored-by: Schuyler Eldridge --- src/test/scala/chiselTests/InvalidateAPISpec.scala | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/InvalidateAPISpec.scala b/src/test/scala/chiselTests/InvalidateAPISpec.scala index 2c51e5d2..dbd353a0 100644 --- a/src/test/scala/chiselTests/InvalidateAPISpec.scala +++ b/src/test/scala/chiselTests/InvalidateAPISpec.scala @@ -228,4 +228,12 @@ class InvalidateAPISpec extends ChiselPropSpec with Matchers with BackendCompila val firrtlOutput = myGenerateFirrtl(new ModuleWithoutDontCare) firrtlOutput should include("is invalid") } + + property("a clock should be able to be connected to a DontCare") { + class ClockConnectedToDontCare extends Module { + val foo = IO(Output(Clock())) + foo := DontCare + } + myGenerateFirrtl(new ClockConnectedToDontCare) should include("foo is invalid") + } } -- cgit v1.2.3 From 70da39e140e96a9302a94864f077529e02596ef5 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 19 Apr 2022 02:29:00 +0000 Subject: Allow creating memories without an implicit clock (#2494) (#2495) Fixes #2470 (cherry picked from commit 44165a259bb16733a41798edca6b554b13f1d54a) Co-authored-by: Kevin Laeufer --- src/test/scala/chiselTests/Mem.scala | 52 ++++++++++++++++++++++++++++++++++++ 1 file changed, 52 insertions(+) (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/Mem.scala b/src/test/scala/chiselTests/Mem.scala index 4dcb1ad4..c5fcc6b1 100644 --- a/src/test/scala/chiselTests/Mem.scala +++ b/src/test/scala/chiselTests/Mem.scala @@ -3,6 +3,7 @@ package chiselTests import chisel3._ +import chisel3.stage.ChiselStage import chisel3.util._ import chisel3.testers.BasicTester @@ -141,6 +142,52 @@ class MemBundleTester extends BasicTester { } } +private class TrueDualPortMemoryIO(val addrW: Int, val dataW: Int) extends Bundle { + require(addrW > 0, "address width must be greater than 0") + require(dataW > 0, "data width must be greater than 0") + + val clka = Input(Clock()) + val ena = Input(Bool()) + val wea = Input(Bool()) + val addra = Input(UInt(addrW.W)) + val dia = Input(UInt(dataW.W)) + val doa = Output(UInt(dataW.W)) + + val clkb = Input(Clock()) + val enb = Input(Bool()) + val web = Input(Bool()) + val addrb = Input(UInt(addrW.W)) + val dib = Input(UInt(dataW.W)) + val dob = Output(UInt(dataW.W)) +} + +private class TrueDualPortMemory(addrW: Int, dataW: Int) extends RawModule { + val io = IO(new TrueDualPortMemoryIO(addrW, dataW)) + val ram = SyncReadMem(1 << addrW, UInt(dataW.W)) + + // Port a + withClock(io.clka) { + io.doa := DontCare + when(io.ena) { + when(io.wea) { + ram(io.addra) := io.dia + } + io.doa := ram(io.addra) + } + } + + // Port b + withClock(io.clkb) { + io.dob := DontCare + when(io.enb) { + when(io.web) { + ram(io.addrb) := io.dib + } + io.dob := ram(io.addrb) + } + } +} + class MemorySpec extends ChiselPropSpec { property("Mem of Vec should work") { assertTesterPasses { new MemVecTester } @@ -186,4 +233,9 @@ class MemorySpec extends ChiselPropSpec { |} |""".stripMargin should compile } + + property("memories in modules without implicit clock should compile without warning or error") { + val stage = new ChiselStage + stage.emitVerilog(new TrueDualPortMemory(4, 32)) + } } -- cgit v1.2.3 From a16a8a52a3b2d72d80a27434217aaeba7be2d3a8 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Wed, 20 Apr 2022 21:10:07 +0000 Subject: Generate a balanced tree with reduceTree (#2318) (#2499) The difference in logic depth for various paths now has a maximum of 1. Also make treeReduce order the same for 2.12 and 2.13 .grouped(_) returns an Iterator .toSeq on an Iterator returns a Stream in 2.12 and a List in 2.13 This can lead to changes in order when bumping from 2.12 to 2.13 that can be avoided by simply using an eager collection explicitly. Co-authored-by: Jack Koenig (cherry picked from commit 6975f77f3325dec46c613552eac663c29011a67c) Co-authored-by: Martin Schoeberl --- src/test/scala/chiselTests/ReduceTreeSpec.scala | 106 ++++++++++++++++++++++++ 1 file changed, 106 insertions(+) create mode 100644 src/test/scala/chiselTests/ReduceTreeSpec.scala (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/ReduceTreeSpec.scala b/src/test/scala/chiselTests/ReduceTreeSpec.scala new file mode 100644 index 00000000..3f078106 --- /dev/null +++ b/src/test/scala/chiselTests/ReduceTreeSpec.scala @@ -0,0 +1,106 @@ +// SPDX-License-Identifier: Apache-2.0 + +package chiselTests + +import chisel3._ +import chisel3.util._ +import chisel3.testers.BasicTester + +class Arbiter[T <: Data: Manifest](n: Int, private val gen: T) extends Module { + val io = IO(new Bundle { + val in = Flipped(Vec(n, new DecoupledIO(gen))) + val out = new DecoupledIO(gen) + }) + + def arbitrateTwo(a: DecoupledIO[T], b: DecoupledIO[T]) = { + + val idleA :: idleB :: hasA :: hasB :: Nil = Enum(4) + val regData = Reg(gen) + val regState = RegInit(idleA) + val out = Wire(new DecoupledIO(gen)) + + a.ready := regState === idleA + b.ready := regState === idleB + out.valid := (regState === hasA || regState === hasB) + + switch(regState) { + is(idleA) { + when(a.valid) { + regData := a.bits + regState := hasA + }.otherwise { + regState := idleB + } + } + is(idleB) { + when(b.valid) { + regData := b.bits + regState := hasB + }.otherwise { + regState := idleA + } + } + is(hasA) { + when(out.ready) { + regState := idleB + } + } + is(hasB) { + when(out.ready) { + regState := idleA + } + } + } + + out.bits := regData.asUInt + 1.U + out + } + + io.out <> io.in.reduceTree(arbitrateTwo) +} + +class ReduceTreeBalancedTester(nodes: Int) extends BasicTester { + + val cnt = RegInit(0.U(8.W)) + val min = RegInit(99.U(8.W)) + val max = RegInit(0.U(8.W)) + + val dut = Module(new Arbiter(nodes, UInt(16.W))) + for (i <- 0 until nodes) { + dut.io.in(i).valid := true.B + dut.io.in(i).bits := 0.U + } + dut.io.out.ready := true.B + + when(dut.io.out.valid) { + val hops = dut.io.out.bits + when(hops < min) { + min := hops + } + when(hops > max) { + max := hops + } + } + + when(!(max === 0.U || min === 99.U)) { + assert(max - min <= 1.U) + } + + cnt := cnt + 1.U + when(cnt === 10.U) { + stop() + } +} + +class ReduceTreeBalancedSpec extends ChiselPropSpec { + property("Tree shall be fair and shall have a maximum difference of one hop for each node") { + + // This test will fail for 5 nodes due to an unbalanced tree. + // A fix is on the way. + for (n <- 1 to 5) { + assertTesterPasses { + new ReduceTreeBalancedTester(n) + } + } + } +} -- cgit v1.2.3 From 3e551ef38fb4c01b9b46f45dc68c2d9f5c9e9acb Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 25 Apr 2022 23:19:37 +0000 Subject: Fix error message for BlackBox without val io <: Record (#2504) (#2505) (cherry picked from commit f9aee1f72744abc6ee13aafc4d1a51a2783cbab8) Co-authored-by: Jack Koenig --- src/test/scala/chiselTests/BlackBox.scala | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) (limited to 'src/test/scala') diff --git a/src/test/scala/chiselTests/BlackBox.scala b/src/test/scala/chiselTests/BlackBox.scala index 27cdbbc4..f923a94a 100644 --- a/src/test/scala/chiselTests/BlackBox.scala +++ b/src/test/scala/chiselTests/BlackBox.scala @@ -145,6 +145,17 @@ class BlackBoxTypeParam(w: Int, raw: String) extends BlackBox(Map("T" -> RawPara }) } +class BlackBoxNoIO extends BlackBox { + // Whoops! typo + val ioo = IO(new Bundle { + val out = Output(UInt(8.W)) + }) +} + +class BlackBoxUIntIO extends BlackBox { + val io = IO(Output(UInt(8.W))) +} + class BlackBoxWithParamsTester extends BasicTester { val blackBoxOne = Module(new BlackBoxConstant(1)) val blackBoxFour = Module(new BlackBoxConstant(4)) @@ -192,7 +203,23 @@ class BlackBoxSpec extends ChiselFlatSpec { assert(DataMirror.modulePorts(m) == Seq("in" -> m.io.in, "out" -> m.io.out)) }) } - "A BlackBoxed using suggestName(\"io\")" should "work (but don't do this)" in { + "A BlackBox using suggestName(\"io\")" should "work (but don't do this)" in { assertTesterPasses({ new BlackBoxTesterSuggestName }, Seq("/chisel3/BlackBoxTest.v"), TesterDriver.verilatorOnly) } + + "A BlackBox with no 'val io'" should "give a reasonable error message" in { + (the[ChiselException] thrownBy { + ChiselStage.elaborate(new Module { + val inst = Module(new BlackBoxNoIO) + }) + }).getMessage should include("must have a port named 'io' of type Record") + } + + "A BlackBox with non-Record 'val io'" should "give a reasonable error message" in { + (the[ChiselException] thrownBy { + ChiselStage.elaborate(new Module { + val inst = Module(new BlackBoxUIntIO) + }) + }).getMessage should include("must have a port named 'io' of type Record") + } } -- cgit v1.2.3