Skip to content

[Buffering][HandshakeToHW] Support unused arrays#748

Merged
zero9178 merged 5 commits intomainfrom
unused-buffer
Feb 26, 2026
Merged

[Buffering][HandshakeToHW] Support unused arrays#748
zero9178 merged 5 commits intomainfrom
unused-buffer

Conversation

@zero9178
Copy link
Copy Markdown
Collaborator

Unused arrays in C previously caused multiple assertions to fail during compilation. The main problem is in the conversion from handshake to HW: Since the memory interface is only generated for memrefs used in a memory interface op, an unused memref would lead to a signature mismatch between the block arguments of the generated hw.module and the handshake.func.

This PR fixes the issue slightly ad-hoc by explicitly handling the case and dropping such memref arguments entirely during the lowering. This also required adjusting the testbench generator since it assumed a memory interface to always be present.

Always generating a memory interface was considered but required much larger changes to the HDL output which seemed more difficult for now.

Fixes #498

Unused arrays in C previously caused multiple assertions to fail during compilation. The main problem is in the conversion from handshake to HW: Since the memory interface is only generated for memrefs used in a memory interface op, an unused memref would lead to a signature mismatch between the block arguments of the generated `hw.module` and the `handshake.func`.

This PR fixes the issue slightly ad-hoc by explicitly handling the case and dropping such memref arguments entirely during the lowering. This also required adjusting the testbench generator since it assumed a memory interface to always be present.

Always generating a memory interface was considered but required much larger changes to the HDL output which seemed more difficult for now.

Fixes #498
@zero9178 zero9178 requested a review from Jiahui17 February 26, 2026 11:40
Copy link
Copy Markdown
Member

@Jiahui17 Jiahui17 left a comment

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we do it in the handshake dialect instead (add a pass HandshakeDropUnusedMemRefArgs or something like that) so it is easier to spot this logic?

PS: we will remove HW from the flow completely at some point in the future

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

That is indeed much simpler 😅 and requires no changes in the testbench generation. PTAL

@Jiahui17
Copy link
Copy Markdown
Member

DynamaticPass is also deprecated, and we want to systematically migrate them back to the plain Pass (again, we don't like inventing unnecessary custom things

Here was the discussion: #284

image

Copy link
Copy Markdown
Member

@Jiahui17 Jiahui17 left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for the fix!

@zero9178 zero9178 merged commit cb8ee24 into main Feb 26, 2026
6 checks passed
Jiahui17 added a commit that referenced this pull request Feb 26, 2026
Making the changes in #748 consistent w.r.t existing code.

Maybe the organization in #748 is simpler and we should do it for all
passes?
@zero9178 zero9178 deleted the unused-buffer branch February 26, 2026 16:28
ziadomalik pushed a commit that referenced this pull request Mar 11, 2026
Unused arrays in C previously caused multiple assertions to fail during
compilation. The main problem is in the conversion from handshake to HW:
Since the memory interface is only generated for memrefs used in a
memory interface op, an unused memref would lead to a signature mismatch
between the block arguments of the generated `hw.module` and the
`handshake.func`.

This PR fixes the issue slightly ad-hoc by explicitly handling the case
and dropping such memref arguments entirely during the lowering. This
also required adjusting the testbench generator since it assumed a
memory interface to always be present.

Always generating a memory interface was considered but required much
larger changes to the HDL output which seemed more difficult for now.

Fixes #498
ziadomalik pushed a commit that referenced this pull request Mar 11, 2026
Making the changes in #748 consistent w.r.t existing code.

Maybe the organization in #748 is simpler and we should do it for all
passes?
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.

Dynamatic crashes with: 42 Segmentation fault

2 participants