Skip to content

[FIRRTLToHW] Fix crash when lowering instance_choice with clock outputs#9941

Draft
seldridge wants to merge 1 commit intomainfrom
dev/augment/fix-lower-to-hw-crash
Draft

[FIRRTLToHW] Fix crash when lowering instance_choice with clock outputs#9941
seldridge wants to merge 1 commit intomainfrom
dev/augment/fix-lower-to-hw-crash

Conversation

@seldridge
Copy link
Copy Markdown
Member

When lowering instance_choice operations with clock-typed output ports,
the code was attempting to create sv.wire operations with !seq.clock as
the element type. However, !seq.clock is not a valid element type for
hw.inout, causing an assertion failure.

The fix follows the same pattern used for XMR operations: use i1 wires
instead of !seq.clock wires, and convert to/from clock when reading/
writing:

  • When creating wires for clock outputs, use i1 type
  • When reading from the wire (for the result value), convert i1 to clock
    using seq.to_clock
  • When writing to the wire (from instance outputs), convert clock to i1
    using seq.from_clock

This allows instance_choice to work correctly with clock-typed ports
while respecting the type constraints of hw.inout.

Fixes a crash with the error:
error: invalid element for hw.inout type '!seq.clock'
Assertion failed: (succeeded(ConcreteT::verifyInvariants(...)))

AI-assisted-by: Augment (Claude Sonnet 4.5)
Signed-off-by: Schuyler Eldridge schuyler.eldridge@sifive.com

This is DRAFT as I have not reviewed it yet!

When lowering instance_choice operations with clock-typed output ports,
the code was attempting to create sv.wire operations with !seq.clock as
the element type. However, !seq.clock is not a valid element type for
hw.inout, causing an assertion failure.

The fix follows the same pattern used for XMR operations: use i1 wires
instead of !seq.clock wires, and convert to/from clock when reading/
writing:
- When creating wires for clock outputs, use i1 type
- When reading from the wire (for the result value), convert i1 to clock
  using seq.to_clock
- When writing to the wire (from instance outputs), convert clock to i1
  using seq.from_clock

This allows instance_choice to work correctly with clock-typed ports
while respecting the type constraints of hw.inout.

Fixes a crash with the error:
  error: invalid element for hw.inout type '!seq.clock'
  Assertion failed: (succeeded(ConcreteT::verifyInvariants(...)))

AI-assisted-by: Augment (Claude Sonnet 4)
Signed-off-by: Schuyler Eldridge <schuyler.eldridge@sifive.com>
@seldridge seldridge requested a review from uenoku March 14, 2026 20:46
@seldridge
Copy link
Copy Markdown
Member Author

There should be a better fix than this once @uenoku finishes instance choice support. I'll leave this open for now.

@uenoku
Copy link
Copy Markdown
Member

uenoku commented Mar 18, 2026

As we don't pursue the direction of #9900 and #9948 for now, we need to fix this even when we switched to generate statement.

Copy link
Copy Markdown
Member

@uenoku uenoku left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants