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] Add generic intrinsic op. #6874

Merged
merged 3 commits into from Mar 27, 2024

Conversation

dtzSiFive
Copy link
Contributor

Add generic intrinsic operation to FIRRTL. Not used anywhere, just adding to the IR to build on.
Re-use module parameter printing/parsing for use as custom printer on the op.

Bit of context:

This is intended to have a FIRRTL textual equivalent that allows intrinsics to be used as expressions or statements, as a replacement for instances of intmodule's in the future, for ergonomic but primarily practical reasons that limit intrinsic use today.

The op is generic so that intrinsics may be parsed without the parser knowing details of supported intrinsics while still having sufficient type information to produce IR and good diagnostics during parsing (return value in particular).

Operands and results are required to be passive (and base) as that's all that is supported today (crashes at various points in the pipeline if not), and because passive specifically fits best with the model of intrinsics as having inputs it reads from and outputs distinctly. This can be loosened in the future should this be needed (and supported).
There is a single (optional) return value, for use as a FIRRTL expression in a single simple format. Multiple outputs become a single result bundle.

cc #6869 which shows some of what's planned next and demonstrates this in practice (internal to CIRCT anyway):
a pass converting instances+intmodules to these operations, and reworking LowerIntrinsics to expect these ops as the canonical form, making it possible to be a module pass and implement lowering as patterns for safety and simpler lowering code.

For now, start by adding the generic intrinsic operation.

@dtzSiFive dtzSiFive added the FIRRTL Involving the `firrtl` dialect label Mar 27, 2024
Copy link
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

@dtzSiFive dtzSiFive merged commit 0cff5d9 into llvm:main Mar 27, 2024
4 checks passed
@dtzSiFive dtzSiFive deleted the feature/generic-intrinsic-op branch March 27, 2024 14:17
@dtzSiFive
Copy link
Contributor Author

Simple op, landing. Post-merge review welcome and of course may change in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FIRRTL Involving the `firrtl` dialect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants