From f2ef3a8ee378a307661bd598cd44d4b895b9352e Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Tue, 8 Nov 2022 07:05:53 +0000 Subject: Improve Record.bind and Detect Records with unstable elements (backport #2829) (#2831) * Add Aggregate.elementsIterator and micro-optimize elementsIterator provides a more efficient API for iterating on the elements of Aggregates. It is especially useful for Records where getElements returns a Seq and thus eagerly constructs a new datastructure which may then just be iterated on anyway. This new elementsIterator API is then used throughout the codebase where it makes sense. Also change Vec.getElements to just return the underlying self instead of constructing a new Seq. (cherry picked from commit defa440b349031475daeff4024fad04925cccee6) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala # core/src/main/scala/chisel3/Module.scala # core/src/main/scala/chisel3/experimental/Trace.scala * Move Aggregate.bind inline into Record.bind Vec overrides bind and does not call the version in Aggregate so the version in Aggregate is misleading in that its only ever used by Records. Now there is no version in Aggregate and the actual functionality and use is more clear. (cherry picked from commit b054c30ba47026cb2a9b28c696a0a0a58b1e2ee7) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala * Extract and optimize duplicate checking Record.bind This replaces an immutable.Map with a single mutable.HashSet and saves the allocation of # elements Seqs. (cherry picked from commit 832ea52bc23424bb75b9654422b725a9cafaef40) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala * Add check for Records that define def elements (cherry picked from commit a4f223415de19e2a732e0b6a8fe681f706a19a56) * Resolve backport conflicts * Make elementsIterator final and package private * Waive false MiMa failure Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Module.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index ba2d2e32..9315a44b 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -709,9 +709,9 @@ package experimental { data match { case record: Record => val compatRecord = !record.compileOptions.dontAssumeDirectionality - record.getElements.foreach(assignCompatDir(_, compatRecord)) + record.elementsIterator.foreach(assignCompatDir(_, compatRecord)) case vec: Vec[_] => - vec.getElements.foreach(assignCompatDir(_, insideCompat)) + vec.elementsIterator.foreach(assignCompatDir(_, insideCompat)) } case SpecifiedDirection.Input | SpecifiedDirection.Output => // forced assign, nothing to do } -- cgit v1.2.3 From be4463a7756351dcab09ba3f576f5e3687fb0ebf Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 10 Nov 2022 20:03:45 +0000 Subject: Unify Chisel2 and chisel3 directionality (backport #2634) (#2837) * Unify Chisel2 and chisel3 directionality (#2634) Co-authored-by: Jack Koenig (cherry picked from commit 1aea4ef96466cbe08150d20c85c88b81e4e4f80f) # Conflicts: # core/src/main/scala/chisel3/Aggregate.scala # core/src/main/scala/chisel3/Module.scala # src/test/scala/chiselTests/Direction.scala * fix up backport * fix up backport * clean up diff * make test order like it was on master Co-authored-by: Adam Izraelevitz Co-authored-by: Megan Wachs --- core/src/main/scala/chisel3/Module.scala | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 9315a44b..48c33083 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -692,33 +692,34 @@ package experimental { */ protected def _bindIoInPlace(iodef: Data): Unit = { // Compatibility code: Chisel2 did not require explicit direction on nodes - // (unspecified treated as output, and flip on nothing was input). - // This sets assigns the explicit directions required by newer semantics on - // Bundles defined in compatibility mode. + // (unspecified treated as output, and flip on nothing was input). + // However, we are going to go back to Chisel2 semantics, so we need to make it work + // even for chisel3 code. + // This assigns the explicit directions required by both semantics on all Bundles. // This recursively walks the tree, and assigns directions if no explicit - // direction given by upper-levels (override Input / Output) AND element is - // directly inside a compatibility Bundle determined by compile options. - def assignCompatDir(data: Data, insideCompat: Boolean): Unit = { + // direction given by upper-levels (override Input / Output) + def assignCompatDir(data: Data): Unit = { data match { - case data: Element if insideCompat => data._assignCompatibilityExplicitDirection - case data: Element => // Not inside a compatibility Bundle, nothing to be done + case data: Element => data._assignCompatibilityExplicitDirection case data: Aggregate => data.specifiedDirection match { // Recurse into children to ensure explicit direction set somewhere case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => data match { case record: Record => - val compatRecord = !record.compileOptions.dontAssumeDirectionality - record.elementsIterator.foreach(assignCompatDir(_, compatRecord)) + record.elementsIterator.foreach(assignCompatDir(_)) case vec: Vec[_] => - vec.elementsIterator.foreach(assignCompatDir(_, insideCompat)) + vec.elementsIterator.foreach(assignCompatDir(_)) } - case SpecifiedDirection.Input | SpecifiedDirection.Output => // forced assign, nothing to do + case SpecifiedDirection.Input | SpecifiedDirection.Output => + // forced assign, nothing to do + // Note this is because Input and Output recurse down their types to align all fields to that SpecifiedDirection + // Thus, no implicit assigment is necessary. } } } - assignCompatDir(iodef, false) + assignCompatDir(iodef) iodef.bind(PortBinding(this)) _ports += iodef -- cgit v1.2.3 From 17c04998d8cd5eeb4eff9506465fd2d6892793d2 Mon Sep 17 00:00:00 2001 From: mergify[bot] Date: Thu, 10 Nov 2022 21:11:55 +0000 Subject: Add unit tests and fix for #2794 , add unit tests for #2773 (backport #2792) (#2834) * Fixup and unit tests for D/I of IOs without explicit Input/Output (#2792) (cherry picked from commit f24a624863f0fc460fd862238688ea8612ffdf5e) # Conflicts: # core/src/main/scala/chisel3/Module.scala * Resolve backport conflicts Co-authored-by: Megan Wachs Co-authored-by: Jack Koenig --- core/src/main/scala/chisel3/Module.scala | 67 ++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 29 deletions(-) (limited to 'core/src/main/scala/chisel3/Module.scala') diff --git a/core/src/main/scala/chisel3/Module.scala b/core/src/main/scala/chisel3/Module.scala index 48c33083..e5c2a848 100644 --- a/core/src/main/scala/chisel3/Module.scala +++ b/core/src/main/scala/chisel3/Module.scala @@ -106,6 +106,38 @@ object Module extends SourceInfoDoc { module } + + /** Assign directionality on any IOs that are still Unspecified/Flipped + * + * Chisel2 did not require explicit direction on nodes + * (unspecified treated as output, and flip on nothing was input). + * As of 3.6, chisel3 is now also using these semantics, so we need to make it work + * even for chisel3 code. + * This assigns the explicit directions required by both semantics on all Bundles. + * This recursively walks the tree, and assigns directions if no explicit + * direction given by upper-levels (override Input / Output) + */ + private[chisel3] def assignCompatDir(data: Data): Unit = { + data match { + case data: Element => data._assignCompatibilityExplicitDirection + case data: Aggregate => + data.specifiedDirection match { + // Recurse into children to ensure explicit direction set somewhere + case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => + data match { + case record: Record => + record.elementsIterator.foreach(assignCompatDir(_)) + case vec: Vec[_] => + vec.elementsIterator.foreach(assignCompatDir(_)) + assignCompatDir(vec.sample_element) // This is used in fromChildren computation + } + case SpecifiedDirection.Input | SpecifiedDirection.Output => + // forced assign, nothing to do + // The .bind algorithm will automatically assign the direction here. + // Thus, no implicit assignment is necessary. + } + } + } } /** Abstract base class for Modules, which behave much like Verilog modules. @@ -382,6 +414,10 @@ package internal { new ClonePorts(proto.getChiselPorts :+ ("io" -> b._io.get): _*) case _ => new ClonePorts(proto.getChiselPorts: _*) } + // getChiselPorts (nor cloneTypeFull in general) + // does not recursively copy the right specifiedDirection, + // still need to fix it up here. + Module.assignCompatDir(clonePorts) clonePorts.bind(PortBinding(cloneParent)) clonePorts.setAllParents(Some(cloneParent)) cloneParent._portsRecord = clonePorts @@ -691,35 +727,8 @@ package experimental { * io, then do operations on it. This binds a Chisel type in-place (mutably) as an IO. */ protected def _bindIoInPlace(iodef: Data): Unit = { - // Compatibility code: Chisel2 did not require explicit direction on nodes - // (unspecified treated as output, and flip on nothing was input). - // However, we are going to go back to Chisel2 semantics, so we need to make it work - // even for chisel3 code. - // This assigns the explicit directions required by both semantics on all Bundles. - // This recursively walks the tree, and assigns directions if no explicit - // direction given by upper-levels (override Input / Output) - def assignCompatDir(data: Data): Unit = { - data match { - case data: Element => data._assignCompatibilityExplicitDirection - case data: Aggregate => - data.specifiedDirection match { - // Recurse into children to ensure explicit direction set somewhere - case SpecifiedDirection.Unspecified | SpecifiedDirection.Flip => - data match { - case record: Record => - record.elementsIterator.foreach(assignCompatDir(_)) - case vec: Vec[_] => - vec.elementsIterator.foreach(assignCompatDir(_)) - } - case SpecifiedDirection.Input | SpecifiedDirection.Output => - // forced assign, nothing to do - // Note this is because Input and Output recurse down their types to align all fields to that SpecifiedDirection - // Thus, no implicit assigment is necessary. - } - } - } - - assignCompatDir(iodef) + // Assign any signals (Chisel or chisel3) with Unspecified/Flipped directions to Output/Input + Module.assignCompatDir(iodef) iodef.bind(PortBinding(this)) _ports += iodef -- cgit v1.2.3