Skip to content

[debug] Inline enum variants into debug var/subfield intrinsics#5420

Open
fkhaidari wants to merge 2 commits into
chipsalliance:mainfrom
fkhaidari:fk-sc/enumdef-inline
Open

[debug] Inline enum variants into debug var/subfield intrinsics#5420
fkhaidari wants to merge 2 commits into
chipsalliance:mainfrom
fkhaidari:fk-sc/enumdef-inline

Conversation

@fkhaidari

@fkhaidari fkhaidari commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Drop the separate circt_debug_enumdef intrinsic. A ChiselEnum-typed circt_debug_var/circt_debug_subfield now carries its variants (JSON) and bit width inline alongside enumTypeName/enumFqn, so every enum use site is self-contained and no per-circuit dedup is needed. This removes an outlier intrinsic and matches the CIRCT side, which resolves enum metadata from the var/subfield itself rather than a staged enumdef table.

Type of Improvement

Interal

Desired Merge Strategy

Rebase: You will rebase the PR onto master and it will be merged with a merge commit.

Release Notes

Drop the separate circt_debug_enumdef intrinsic. A ChiselEnum-typed circt_debug_var/circt_debug_subfield now carries its variants (JSON) and bit width inline alongside enumTypeName/enumFqn

@fkhaidari fkhaidari force-pushed the fk-sc/enumdef-inline branch from 15c2212 to 09cab64 Compare June 17, 2026 13:44
@fkhaidari

Copy link
Copy Markdown
Contributor Author

Hi @jackkoenig!

I’ve reworked the debug intrinsic design to cut down on the translation complexity on the CIRCT side.

The separate circt_debug_enumdef intrinsic is gone. A ChiselEnum-typed circt_debug_var/circt_debug_subfield now carries its variants (as JSON) and bit width inline, next to enumTypeName/enumFqn. Every enum use site is self-contained, so there’s no per-circuit dedup pass and nothing for CIRCT to resolve against a staged enumdef table; the enum metadata travels with the value.

The matching CIRCT change reads the variants straight off the var/subfield and materializes the dbg.enum at the use site. I’ll push that side once this lands so the two stay in sync.

Could you take a look?

@fkhaidari fkhaidari force-pushed the fk-sc/enumdef-inline branch from 09cab64 to 9fd285f Compare June 17, 2026 14:16
@fkhaidari

fkhaidari commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

While I’m here, I’d like your take on one thing

For a bidirectional or aggregate signal, circt_debug_var is emitted with no SSA operand, so CIRCT has to recover the root signal by matching the var’s name against the wires/ports/registers in the module (resolveRootSignal in llvm/circt#10647). That name-based lookup is the fragile part: it has to disambiguate by name and special-case memories, and it breaks down whenever PrettifyVerilogNames or lowering renames the underlying signal.

The reason it can’t just take the signal as an operand is that firrtl.int.generic operands must be passive (FIRRTLIntrinsics.td), so a bidirectional value can’t be passed through. Is there a way to attach the signal as a proper operand (or otherwise give CIRCT a stable handle) that I’m missing, or is the name-based binding the intended contract here?

@fkhaidari fkhaidari force-pushed the fk-sc/enumdef-inline branch from 9fd285f to 774bf0a Compare June 18, 2026 08:51
@fkhaidari fkhaidari changed the title draft: [debug] Inline enum variants into debug var/subfield intrinsics [debug] Inline enum variants into debug var/subfield intrinsics Jun 18, 2026
Drop the separate circt_debug_enumdef intrinsic. A ChiselEnum-typed
circt_debug_var/circt_debug_subfield now carries its variants (JSON) and
bit width inline alongside enumTypeName/enumFqn, so every enum use site is
self-contained and no per-circuit dedup is needed. This removes an outlier
intrinsic and matches the CIRCT side, which resolves enum metadata from the
var/subfield itself rather than a staged enumdef table.

Signed-off-by: fkhaidari <khaidari.fg@gmail.com>
@fkhaidari fkhaidari force-pushed the fk-sc/enumdef-inline branch from 774bf0a to b52be31 Compare June 18, 2026 21:38
@fkhaidari

Copy link
Copy Markdown
Contributor Author

@jackkoenig, could you take a look?

@jackkoenig jackkoenig left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without being an expert on the CIRCT representation, this makes sense to me and sounds a lot better.

@jackkoenig jackkoenig added the Backend Code Generation Affects backend code generation, will be included in release notes label Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Code Generation Affects backend code generation, will be included in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants