Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIRRTL][LowerDPI] Lower FIRRTL vector to an open array type #7305

Merged
merged 1 commit into from
Aug 5, 2024

Conversation

uenoku
Copy link
Member

@uenoku uenoku commented Jul 11, 2024

Previously a FIRRTL vector was lowered into a packed array and there was no way
to generate an open array. This PR chagnes to use unpacked open array which is
supported by several tools (at least verilator and vcs)

Base automatically changed from dev/hidetou/open-array-e2e to main July 22, 2024 18:06
@uenoku uenoku force-pushed the dev/hidetou/dpi-firrtl-intrinsic branch 2 times, most recently from b15d9c0 to eb70a6d Compare August 2, 2024 07:22
Previously a FIRRTL vector was lowered into a packed array and there was no way
to generate an open array. This PR chagnes to use unpacked open array which is
supported by several tools (at least verilator and vcs)
@uenoku uenoku force-pushed the dev/hidetou/dpi-firrtl-intrinsic branch from eb70a6d to bc8cd49 Compare August 2, 2024 07:42
@uenoku uenoku changed the title [FIRRTL] Add a intrinsic for defining DPI argument with open array type [FIRRTL][LowerDPI] Lower FIRRTL vector to an open array type Aug 2, 2024
@uenoku uenoku marked this pull request as ready for review August 2, 2024 08:18
Copy link
Contributor

@fabianschuiki fabianschuiki left a comment

Choose a reason for hiding this comment

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

I think this makes sense!

If I recall correctly, passing packed arrays through DPI directly can have weird memory layouts that makes it difficult to extract elements on the C side (because of the bit-dense packing in SV). Making arrays always go through an open array conversion where the C side can easily reason about the dimensions sounds like a good idea 👍

@sequencer
Copy link
Contributor

Looking forward to adopt this PR:) wish it can be used for deduping a massive DPI call of interface with different parameters.

@uenoku uenoku merged commit 461c631 into main Aug 5, 2024
4 checks passed
@uenoku uenoku deleted the dev/hidetou/dpi-firrtl-intrinsic branch August 5, 2024 03:21
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.

3 participants