Skip to content

Commit 531dd6c

Browse files
authored
Fix for 792 (#793)
Makes Builder.updateBundleStack a bit stricter in deciding how many stack frames to discard by additionally matching against method names and deleting stack frames at or above the frame currently being inserted.
1 parent 4655343 commit 531dd6c

File tree

2 files changed

+29
-9
lines changed

2 files changed

+29
-9
lines changed

chiselFrontend/src/main/scala/chisel3/internal/Builder.scala

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,8 @@ private[chisel3] class DynamicContext() {
173173
var currentClockAndReset: Option[ClockAndReset] = None
174174
val errors = new ErrorLog
175175
val namingStack = new internal.naming.NamingStack
176-
// Record the Bundle instance, class name, and reverse stack trace position of open Bundles
177-
val bundleStack: ArrayBuffer[(Bundle, String, Int)] = ArrayBuffer()
176+
// Record the Bundle instance, class name, method name, and reverse stack trace position of open Bundles
177+
val bundleStack: ArrayBuffer[(Bundle, String, String, Int)] = ArrayBuffer()
178178
}
179179

180180
private[chisel3] object Builder {
@@ -247,24 +247,29 @@ private[chisel3] object Builder {
247247
// Returns the current stack of open Bundles
248248
// Note: elt will NOT have finished construction, its elements cannot be accessed
249249
def updateBundleStack(elt: Bundle): Seq[Bundle] = {
250-
val stackClasses = Thread.currentThread().getStackTrace()
251-
.map(_.getClassName)
250+
val stackElts = Thread.currentThread().getStackTrace()
252251
.reverse // so stack frame numbers are deterministic across calls
252+
.dropRight(2) // discard Thread.getStackTrace and updateBundleStack
253+
254+
// Determine where we are in the Bundle stack
255+
val eltClassName = elt.getClass.getName
256+
val eltStackPos = stackElts.map(_.getClassName).lastIndexOf(eltClassName)
253257

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

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

263269
// Append the current Bundle to the stack, if it's on the stack trace
264-
val eltClassName = elt.getClass.getName
265-
val eltStackPos = stackClasses.lastIndexOf(eltClassName)
266270
if (eltStackPos >= 0) {
267-
dynamicContext.bundleStack.append((elt, eltClassName, eltStackPos))
271+
val stackElt = stackElts(eltStackPos)
272+
dynamicContext.bundleStack.append((elt, eltClassName, stackElt.getMethodName, eltStackPos))
268273
}
269274
// Otherwise discard the stack frame, this shouldn't fail noisily
270275

src/test/scala/chiselTests/AutoClonetypeSpec.scala

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ object CompanionObjectWithBundle {
5656
}
5757
}
5858

59+
class NestedAnonymousBundle extends Bundle {
60+
val a = Output(new Bundle {
61+
val a = UInt(8.W)
62+
})
63+
}
64+
5965
// A Bundle with an argument that is also a field.
6066
// Not necessarily good style (and not necessarily recommended), but allowed to preserve compatibility.
6167
class BundleWithArgumentField(val x: Data, val y: Data) extends Bundle
@@ -146,4 +152,13 @@ class AutoClonetypeSpec extends ChiselFlatSpec {
146152
io.data := 1.U
147153
} }
148154
}
155+
156+
"Nested directioned anonymous Bundles" should "not need clonetype" in {
157+
elaborate { new Module {
158+
val io = IO(new NestedAnonymousBundle)
159+
val a = WireInit(io)
160+
io.a.a := 1.U
161+
} }
162+
}
163+
149164
}

0 commit comments

Comments
 (0)