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][ExportVerilog] Emit integers on DPI function as two state C-compatible types and clarify ABI #7163

Merged
merged 1 commit into from
Jun 13, 2024

Conversation

uenoku
Copy link
Member

@uenoku uenoku commented Jun 12, 2024

This PR modifies ExportVerilog to emit two state types (bit in general) for DPI import op. Furthermore, for specific bit width (8, 16, 32 and 64) it emits C-types (byte, shortint, int and longint).

This PR also rejects small integer types other than 8, 16, 32 and 64 bit width since otherwise we have to usebit but they are passed by references in DPI. So this PR defines the ABI as "small integers are passed by values, and larger integers are passed by references.

@uenoku uenoku requested review from fabianschuiki and dtzSiFive and removed request for seldridge, darthscsi and dtzSiFive June 12, 2024 13:55
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 love the idea of limiting arguments to the types which get passed by-value according to the SV spec. We can always make this more general later. 🥳

@uenoku uenoku changed the title [FIRTRL][ExportVerilog] Emit integers on DPI function as two state C-compatible types and clarify ABI [FIRRTL][ExportVerilog] Emit integers on DPI function as two state C-compatible types and clarify ABI Jun 13, 2024
Base automatically changed from dev/hidetou/lower-dpi to main June 13, 2024 11:54
@uenoku uenoku merged commit 523e9b4 into main Jun 13, 2024
4 checks passed
@uenoku uenoku deleted the dev/hidetou/integeration branch June 13, 2024 14:08
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