[Debug][FIRRTL][2/13] Lower circt_debug_* intrinsics to Debug ops

[fkhaidari]

Convert circt_debug_var, circt_debug_subfield, and circt_debug_enumdef into Debug-dialect ops carrying source-language type metadata.

A pre-pass stages subfield leaves and enumdef variant data (keyed by FQN), then erases both, so the converters stay stateless. Linkage is by FQN: a subfield's parent names the root variable and name is its dotted display path. Enum use sites build a dbg.enum inline from the staged variants, so no separate definition op is needed. Leaf matching descends bundles and vectors and treats FIRRTL enum fields as leaves; a dangling FQN or unmatched leaf is a warning, not an error.

[fkhaidari]

Hey @fabianschuiki, @uenoku, @darthscsi, @seldridge! In this PR, I'm continuing my work on the debug stack that was started in #10579. The first user for new dbg operations is appearing here. New intrinsics was introduced in Chisel v7.13.0. And do not be afraid of diff size. Most of it are tests

[circt-bot]

Results of circt-tests run for 46e30e2 compared to results for e003d2c: no change to test results.

[uenoku]

I haven't looked at the detail yet but:

1. Why can't we reuse/extend https://github.com/llvm/circt/blob/main/lib/Dialect/FIRRTL/Transforms/MaterializeDebugInfo.cpp?
2. Why is a huge framework change for LowerIntrinsic necessary? I think we would be more comfortable if the change is simply adding intrinsics ... Especially representation of debug_enumdef is very outlier compared to other intrinsics. Can we embed that information to the debug_var or subfield intrinsic itself?
3. nit: Could you consider using tablegen for instrinsic definition?
4. resolveRootSignal feels very fragile and I'm not sure it's going to work ... it looks encoding another SSA edge into intrinsic's field. Can't we pass these values to intrinsics simply?

[fkhaidari]

Thanks @uenoku, going in order:

1. Reuse MaterializeDebugInfo? Different layers: it emits basic dbg.variables for uncovered signals, the intrinsic lowering emits the rich variant (typeName, enum, params). Order is pinned and it skips intrinsic-covered signals, so no duplicates. Could fold them, but that's a refactor, not a simplification, and point 2 removes most of the extra framework anyway.

2. enumdef is an outlier, embed it? Agreed, will do. Inline the variants into circt_debug_var/subfield (next to enumFqn) and drop circt_debug_enumdef. That kills the pre-pass, the enumDefByFqn table, and the lookup. EmitUHDI already interns by FQN from the inline dbg.enum, so output is unchanged.

3. tablegen (nit). As far as I know there is no tablegen path for intrinsic converters today; all ~18 are registered by hand via lowering.add<...>. Open to a tablegen migration, but that's a separate framework-wide change.

4. resolveRootSignal is fragile, pass operands instead? We do, everywhere except two cases with no passive SSA root: memories (no single SSA value) and bidirectional aggregate roots (int.generic operands must be passive). A passive copy drops flipped fields, and the aggregate is rebuilt by descending the root's type, so dropping the root loses uncovered/intermediate fields in nested bundles. So it's covering a real gap, not encoding an SSA edge we could've passed.

@jackkoenig, both of these (1 and 4) touch how Chisel emits the debug intrinsics, so wanted your input. For point 2 I'm planning to inline the enum variants into circt_debug_var/subfield and drop the separate circt_debug_enumdef; flag me if that's a problem on the frontend side. Point 4 is the open one: for a bidirectional aggregate root, int.generic needs a passive operand, and a plain passive copy loses the flipped fields. Is there a clean way in Chisel to hand the backend a passive projection of the root that keeps both directions, or do you think the by-name fallback is the better trade-off for these two cases?

Plan: do point 2 now, settle point 4 with Jack first.

[jackkoenig]

Point 4 is the open one: for a bidirectional aggregate root, int.generic needs a passive operand, and a plain passive copy loses the flipped fields. Is there a clean way in Chisel to hand the backend a passive projection of the root that keeps both directions, or do you think the by-name fallback is the better trade-off for these two cases?

We definitely want to avoid doing anything by name.

2.5 possibilities come to mind:

1. I don't see why a passive copy needs to lose the flipped fields, e.g. in Chisel you can write:

val monitor = Wire(Output(someBidirectionalThing)) // Output coerces all directions to aligned
monitor :#= someBidirectionalThing

Can we do something like that? It depends on what the operation is trying to do of course.

2. Rather than taking a single operand, you could splat the bidirectional type to its leaves and track all of them

3. (Really 2.5) You could do a hybrid-y approach where you don't splat leaves but you splat down to aggregates that are themselves passive.

[fkhaidari]

Thanks @jackkoenig! I prototyped both option 1 and option 2.5 end-to-end and compared emitted Verilog and debug output against the by-name baseline.

Option 1 works: with a droppable _-prefixed name the wire dissolves during hw.wire canonicalization, Verilog is byte-identical, and every leaf resolves to the original names. Two costs: our debug passes that snapshot the FIRRTL statement stream still see the wire's decls/connects and would need filtering, and correctness rests on the canonicalizer folding the wire, the same kind of implicit invariant this review started from. (Also, the intrinsics are emitted post-elaboration, so no Wire/:#=; the wire has to be raw IR.)

Option 2.5 came out cleaner: the maximal flip-free subtrees are already passive expressions, so the frontend just passes them as operands plus their paths in an optional operandPaths parameter, and the converter stitches them
back. Both Verilog and debug output are byte-identical to baseline, and mismatches fail with diagnostics. Going with this one; a Vec of bidirectional elements keeps the no-operand fallback for now.

So the plan: implement option 2.5 in this PR and delete resolveRootSignal. @jackkoenig, @uenoku are you OK with this direction?

[fabianschuiki]

Hey @fkhaidari! Really cool that you are building this stuff out 🥳. One thing I'm wondering: why do you need things like enumdef? Since you are essentially attaching debug information to an SSA value, I don't think there's any need for a definition/declaration indirection. Could you have the debug var intrinsic, or some debug enum value intrinsic, carry the exact layout of the enum as an attribute?

It would be great to have a chat or discussion about how to represent this in CIRCT. Looking at some of the code, I feel like most of this has been driven by what the easiest way to emit some of this stuff in Chisel is. Chisel is very strange and quirky though, and we'd probably rather want to have the compiler itself have a robust idiomatic representation for the debug info and then do the heavy lifting on how to generate that on the Chisel side.

Do you have a concrete example of what exactly you would like to preserve from Chisel all the way to the output? I suspect that you'll need very little compiler machinery -- only something like a dbg.enum op to ascribe an enum type to an SSA value. All the other stuff, enumdef, subfields, etc. feels like it's a consequence of Chisel's weird connect semantics, but you'd actually just use dbg.array and dbg.struct to reconstruct the aggregates. No need for subfields. You use dbg.enum and other type metadata ops on the individual SSA values corresponding to the subfields, and then aggregate them back up with dbg.array and dbg.struct.

[fkhaidari]

Hey @fabianschuiki, thanks for taking a look!

enumdef: agreed, and it's already on its way out on both levels. At the dialect level the definition/declaration indirection is gone since the #10579 review: dbg.enum carries the layout inline as an attribute, exactly as you suggest, and this PR realises one at each use site. The circt_debug_enumdef intrinsic you see here is the last remnant: as agreed with @uenoku above, the variants get inlined into circt_debug_var/circt_debug_subfield and the separate intrinsic disappears; that change lands in the next push. The "dbg.enumdef" mentions in the test comments are stale wording from before #10579 and go away with it.

The direction: the goal is one structured debug-info document (UHDI) that opensource debugging tools like hgdb, tywaves, and chiseltrace consume. Two kinds of information have to survive from the frontend to that output. First, named source values with their source types: aggregates, enums, type parameters. There you're right that very little machinery is needed: everything lowers into the ops from #10579 (dbg.variable + dbg.struct/dbg.array, dbg.enum, dbg.value), and the subfield intrinsic is only the flat transport that the converter stitches back before anything downstream sees it. Second, where statements sit and under which guards. Breakpoints and stepping need that, value tracking can't express it, and that's the one real dialect addition later in the stack (a statement tree: dbg.rootblock/dbg.subblock/dbg.connect_stmt).

Example: an end-to-end sample (ChiselEnum FSM through firtool --emit-uhdi): https://github.com/fkhaidari/uhdi/tree/main/demo/fsm

Chat: gladly. There's an RFC with the full picture: https://discourse.llvm.org/t/uhdi-structured-debug-info-export-for-chisel-firrtl/90973, happy to discuss there, and I can add an agenda item for the July 15 ODM.