From 23ef9aa7ffef5bbf8fe124fc9be7683f005c3612 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 16 Aug 2022 19:04:28 +0000 Subject: Add OpaqueType support to Records (backport #2662) (#2679) * Add OpaqueType support to Records (#2662) OpaqueTypes are essentially type aliases that hide the underlying type. They are implemented in Chisel as Records of a single, unnamed element where the wrapping Record does not exist in the emitted FIRRTL. Co-authored-by: Jack Koenig (cherry picked from commit df5afee2d41b5bcd82d4013b977965c0dfe046fd) * Fix test compilation * Fix OpaqueType tests in RecordSpec Need to implement cloneType correctly and to warn instead of error when accessing .toTarget of non-hardware types because that is a warning (not error) in 3.5. * Waive MiMa false positives Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com> Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 10 ++++++++-- core/src/main/scala/chisel3/internal/firrtl/IR.scala | 6 ++++++ 2 files changed, 14 insertions(+), 2 deletions(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 2d21a7cb..56422b85 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -68,6 +68,8 @@ private[chisel3] object Converter { fir.Reference(name, fir.UnknownType) case Slot(imm, name) => fir.SubField(convert(imm, ctx, info), name, fir.UnknownType) + case OpaqueSlot(imm, name) => + convert(imm, ctx, info) case Index(imm, ILit(idx)) => fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType) case Index(imm, value) => @@ -301,7 +303,7 @@ private[chisel3] object Converter { case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint) case d: Analog => fir.AnalogType(convert(d.width)) case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length) - case d: Record => + case d: Record => { val childClearDir = clearDir || d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output def eltField(elt: Data): fir.Field = (childClearDir, firrtlUserDirOf(elt)) match { @@ -311,7 +313,11 @@ private[chisel3] object Converter { case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) => fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info)) } - fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + if (!d.opaqueType) + fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) + else + extractType(d.elements.head._2, true, info) + } } def convert(name: String, param: Param): fir.Param = param match { diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index dc9ab027..37fb2f8b 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -91,6 +91,7 @@ object Arg { case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]" case Some(Index(Node(imm), arg)) => s"${earlyLocalName(imm)}[${arg.localName}]" case Some(Slot(Node(imm), name)) => s"${earlyLocalName(imm)}.$name" + case Some(OpaqueSlot(Node(imm), name)) => s"${earlyLocalName(imm)}" case Some(arg) => arg.name case None => id match { @@ -218,6 +219,11 @@ case class Slot(imm: Node, name: String) extends Arg { } } +case class OpaqueSlot(imm: Node, name: String) extends Arg { + override def contextualName(ctx: Component): String = imm.name + override def localName: String = imm.name +} + case class Index(imm: Arg, value: Arg) extends Arg { def name: String = s"[$value]" override def contextualName(ctx: Component): String = s"${imm.contextualName(ctx)}[${value.contextualName(ctx)}]" -- cgit v1.2.3 From 998913f9379440db26b6aeeaa09e7a11d7615351 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 25 Aug 2022 18:14:54 +0000 Subject: Bugfix - OpaqueSlot replace invalid localName (backport #2701) (#2702) * Bugfix - OpaqueSlot replace invalid localName (#2701) (cherry picked from commit fb8ea2a2fac227f2570da992d7877de2eb1cf801) * Fix cloneTypes (#2703) Co-authored-by: Aditya Naik <91489422+adkian-sifive@users.noreply.github.com>--- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 2 +- core/src/main/scala/chisel3/internal/firrtl/IR.scala | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 56422b85..fe95445c 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -68,7 +68,7 @@ private[chisel3] object Converter { fir.Reference(name, fir.UnknownType) case Slot(imm, name) => fir.SubField(convert(imm, ctx, info), name, fir.UnknownType) - case OpaqueSlot(imm, name) => + case OpaqueSlot(imm) => convert(imm, ctx, info) case Index(imm, ILit(idx)) => fir.SubIndex(convert(imm, ctx, info), castToInt(idx, "Index"), fir.UnknownType) diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index 37fb2f8b..d177c859 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -91,7 +91,7 @@ object Arg { case Some(Index(Node(imm), Node(value))) => s"${earlyLocalName(imm)}[${earlyLocalName(imm)}]" case Some(Index(Node(imm), arg)) => s"${earlyLocalName(imm)}[${arg.localName}]" case Some(Slot(Node(imm), name)) => s"${earlyLocalName(imm)}.$name" - case Some(OpaqueSlot(Node(imm), name)) => s"${earlyLocalName(imm)}" + case Some(OpaqueSlot(Node(imm))) => s"${earlyLocalName(imm)}" case Some(arg) => arg.name case None => id match { @@ -219,9 +219,9 @@ case class Slot(imm: Node, name: String) extends Arg { } } -case class OpaqueSlot(imm: Node, name: String) extends Arg { +case class OpaqueSlot(imm: Node) extends Arg { override def contextualName(ctx: Component): String = imm.name - override def localName: String = imm.name + override def name: String = imm.name } case class Index(imm: Arg, value: Arg) extends Arg { -- cgit v1.2.3 From 9f1484572e2e4185e87a9cfb03b253870636c12c Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 29 Aug 2022 21:05:56 +0000 Subject: Fix OpaqueSlot handling of contextual names (#2708) (#2712) We need to ensure that contextual names stay contextual (ie. sensitive to the module context which is important for naming ports). (cherry picked from commit cee255216c4a1bb658a2d8ddc03d966ce7ffb877) Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/firrtl/IR.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/IR.scala b/core/src/main/scala/chisel3/internal/firrtl/IR.scala index d177c859..ddad6b10 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/IR.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/IR.scala @@ -220,7 +220,7 @@ case class Slot(imm: Node, name: String) extends Arg { } case class OpaqueSlot(imm: Node) extends Arg { - override def contextualName(ctx: Component): String = imm.name + override def contextualName(ctx: Component): String = imm.contextualName(ctx) override def name: String = imm.name } -- cgit v1.2.3 From 76ada881d077118384907f498576b3b338291ff6 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Mon, 7 Nov 2022 19:13:49 +0000 Subject: Bugfix converter clearing flips (backport #2788) (#2832) * Bugfix converter clearing flips (#2788) * Bugfix: Output on Vec of bundle with mixed field orientations * Bugfix OpaqueTypes clearing flips (cherry picked from commit f05bff1a337589bafebd08783bb0f6a72092a95a) # Conflicts: # src/test/scala/chiselTests/Direction.scala * Resolve backport conflicts Co-authored-by: Adam Izraelevitz Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index fe95445c..f73e85d2 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -301,8 +301,11 @@ private[chisel3] object Converter { case d: SInt => fir.SIntType(convert(d.width)) case d: FixedPoint => fir.FixedType(convert(d.width), convert(d.binaryPoint)) case d: Interval => fir.IntervalType(d.range.lowerBound, d.range.upperBound, d.range.firrtlBinaryPoint) - case d: Analog => fir.AnalogType(convert(d.width)) - case d: Vec[_] => fir.VectorType(extractType(d.sample_element, clearDir, info), d.length) + case d: Analog => fir.AnalogType(convert(d.width)) + case d: Vec[_] => + val childClearDir = clearDir || + d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output + fir.VectorType(extractType(d.sample_element, childClearDir, info), d.length) case d: Record => { val childClearDir = clearDir || d.specifiedDirection == SpecifiedDirection.Input || d.specifiedDirection == SpecifiedDirection.Output @@ -316,7 +319,7 @@ private[chisel3] object Converter { if (!d.opaqueType) fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) else - extractType(d.elements.head._2, true, info) + extractType(d.elements.head._2, childClearDir, info) } } -- cgit v1.2.3 From bfa9f7465e6069b1e624126f9e14245b69e7c0a9 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 8 Nov 2022 17:27:07 +0000 Subject: Switch to using experimental trait for OpaqueTypes (backport #2783) (#2836) * Switch to using experimental trait for OpaqueTypes (#2783) This makes it more clear that the feature is experimental. Users may still override the opaqueType method for more dynamic control over when instances of a given Record are OpaqueTypes or not, but they are discouraged from doing so. (cherry picked from commit 7525dc71ccc2050d8e4a68b38f3b76920ba693fc) * Fix cloneType in RecordSpec Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/internal/firrtl/Converter.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index f73e85d2..3d6e0d79 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -316,7 +316,7 @@ private[chisel3] object Converter { case (false, SpecifiedDirection.Flip | SpecifiedDirection.Input) => fir.Field(getRef(elt, info).name, fir.Flip, extractType(elt, false, info)) } - if (!d.opaqueType) + if (!d._isOpaqueType) fir.BundleType(d.elements.toIndexedSeq.reverse.map { case (_, e) => eltField(e) }) else extractType(d.elements.head._2, childClearDir, info) -- cgit v1.2.3 From c8046636a25474be4c547c6fe9c6d742ea7b1d13 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Fri, 11 Nov 2022 01:40:53 +0000 Subject: Change RawModule._commands to a VectorBuilder (backport #2839) (#2841) * Change RawModule._commands to a VectorBuilder (#2839) * Change RawModule._commands to a VectorBuilder Use the resulting Vector to build the underlying Component's commands and then use those instead of copying the original ArrayBuffer when iterating on commands. Previously, the Component was using a List to hold the commands which is particularly memory inefficient, especially for large modules. * Optimize Converter's handling of Seq[Command] It previously converted the Commands to a List (which, while not captured in the type system, they were already a List) and then used head and tail iteration. This is less efficient with the new underlying Vector implementation. (cherry picked from commit 48a1ef0a3872c6b68d46145764d977926923a270) * Waive false binary compatibility failures Co-authored-by: Jack Koenig Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>--- .../scala/chisel3/internal/firrtl/Converter.scala | 116 ++++++++++++--------- 1 file changed, 68 insertions(+), 48 deletions(-) (limited to 'core/src/main/scala/chisel3/internal/firrtl') diff --git a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala index 3d6e0d79..ca39608f 100644 --- a/core/src/main/scala/chisel3/internal/firrtl/Converter.scala +++ b/core/src/main/scala/chisel3/internal/firrtl/Converter.scala @@ -8,7 +8,7 @@ import firrtl.{ir => fir} import chisel3.internal.{castToInt, throwException, HasId} import scala.annotation.{nowarn, tailrec} -import scala.collection.immutable.Queue +import scala.collection.immutable.{Queue, VectorBuilder} import scala.collection.immutable.LazyList // Needed for 2.12 alias @nowarn("msg=class Port") // delete when Port becomes private @@ -215,7 +215,7 @@ private[chisel3] object Converter { * @param alt Indicates if currently processing commands in the alternate (else) of the when scope */ // TODO we should probably have a different structure in the IR to close elses - private case class WhenFrame(when: fir.Conditionally, outer: Queue[fir.Statement], alt: Boolean) + private case class WhenFrame(when: fir.Conditionally, outer: VectorBuilder[fir.Statement], alt: Boolean) /** Convert Chisel IR Commands into FIRRTL Statements * @@ -226,52 +226,72 @@ private[chisel3] object Converter { * @return FIRRTL Statement that is equivalent to the input cmds */ def convert(cmds: Seq[Command], ctx: Component): fir.Statement = { - @tailrec - def rec(acc: Queue[fir.Statement], scope: List[WhenFrame])(cmds: Seq[Command]): Seq[fir.Statement] = { - if (cmds.isEmpty) { - assert(scope.isEmpty) - acc - } else - convertSimpleCommand(cmds.head, ctx) match { - // Most Commands map 1:1 - case Some(stmt) => - rec(acc :+ stmt, scope)(cmds.tail) - // When scoping logic does not map 1:1 and requires pushing/popping WhenFrames - // Please see WhenFrame for more details - case None => - cmds.head match { - case WhenBegin(info, pred) => - val when = fir.Conditionally(convert(info), convert(pred, ctx, info), fir.EmptyStmt, fir.EmptyStmt) - val frame = WhenFrame(when, acc, false) - rec(Queue.empty, frame +: scope)(cmds.tail) - case WhenEnd(info, depth, _) => - val frame = scope.head - val when = - if (frame.alt) frame.when.copy(alt = fir.Block(acc)) - else frame.when.copy(conseq = fir.Block(acc)) - // Check if this when has an else - cmds.tail.headOption match { - case Some(AltBegin(_)) => - assert(!frame.alt, "Internal Error! Unexpected when structure!") // Only 1 else per when - rec(Queue.empty, frame.copy(when = when, alt = true) +: scope.tail)(cmds.drop(2)) - case _ => // Not followed by otherwise - // If depth > 0 then we need to close multiple When scopes so we add a new WhenEnd - // If we're nested we need to add more WhenEnds to ensure each When scope gets - // properly closed - val cmdsx = if (depth > 0) WhenEnd(info, depth - 1, false) +: cmds.tail else cmds.tail - rec(frame.outer :+ when, scope.tail)(cmdsx) - } - case OtherwiseEnd(info, depth) => - val frame = scope.head - val when = frame.when.copy(alt = fir.Block(acc)) - // TODO For some reason depth == 1 indicates the last closing otherwise whereas - // depth == 0 indicates last closing when - val cmdsx = if (depth > 1) OtherwiseEnd(info, depth - 1) +: cmds.tail else cmds.tail - rec(scope.head.outer :+ when, scope.tail)(cmdsx) - } - } + var stmts = new VectorBuilder[fir.Statement]() + var scope: List[WhenFrame] = Nil + var cmdsIt = cmds.iterator.buffered + // Extra var because sometimes we want to push a Command to the head of cmdsIt + // This is more efficient than changing the iterator + var nextCmd: Command = null + while (nextCmd != null || cmdsIt.hasNext) { + val cmd = if (nextCmd != null) { + val _nextCmd = nextCmd + nextCmd = null + _nextCmd + } else { + cmdsIt.next() + } + convertSimpleCommand(cmd, ctx) match { + // Most Commands map 1:1 + case Some(stmt) => + stmts += stmt + // When scoping logic does not map 1:1 and requires pushing/popping WhenFrames + // Please see WhenFrame for more details + case None => + cmd match { + case WhenBegin(info, pred) => + val when = fir.Conditionally(convert(info), convert(pred, ctx, info), fir.EmptyStmt, fir.EmptyStmt) + val frame = WhenFrame(when, stmts, false) + stmts = new VectorBuilder[fir.Statement] + scope = frame :: scope + case WhenEnd(info, depth, _) => + val frame = scope.head + val when = + if (frame.alt) frame.when.copy(alt = fir.Block(stmts.result())) + else frame.when.copy(conseq = fir.Block(stmts.result())) + // Check if this when has an else + cmdsIt.headOption match { + case Some(AltBegin(_)) => + assert(!frame.alt, "Internal Error! Unexpected when structure!") // Only 1 else per when + scope = frame.copy(when = when, alt = true) :: scope.tail + cmdsIt.next() // Consume the AltBegin + stmts = new VectorBuilder[fir.Statement] + case _ => // Not followed by otherwise + // If depth > 0 then we need to close multiple When scopes so we add a new WhenEnd + // If we're nested we need to add more WhenEnds to ensure each When scope gets + // properly closed + if (depth > 0) { + nextCmd = WhenEnd(info, depth - 1, false) + } + stmts = frame.outer + stmts += when + scope = scope.tail + } + case OtherwiseEnd(info, depth) => + val frame = scope.head + val when = frame.when.copy(alt = fir.Block(stmts.result())) + // TODO For some reason depth == 1 indicates the last closing otherwise whereas + // depth == 0 indicates last closing when + if (depth > 1) { + nextCmd = OtherwiseEnd(info, depth - 1) + } + stmts = frame.outer + stmts += when + scope = scope.tail + } + } } - fir.Block(rec(Queue.empty, List.empty)(cmds)) + assert(scope.isEmpty) + fir.Block(stmts.result()) } def convert(width: Width): fir.Width = width match { @@ -347,7 +367,7 @@ private[chisel3] object Converter { def convert(component: Component): fir.DefModule = component match { case ctx @ DefModule(_, name, ports, cmds) => - fir.Module(fir.NoInfo, name, ports.map(p => convert(p)), convert(cmds.toList, ctx)) + fir.Module(fir.NoInfo, name, ports.map(p => convert(p)), convert(cmds, ctx)) case ctx @ DefBlackBox(id, name, ports, topDir, params) => fir.ExtModule( fir.NoInfo, -- cgit v1.2.3