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

Extend the TfheRustBool Emitter with the Packed API #945

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WoutLegiest
Copy link
Collaborator

This PR is a follow up of #842
We implemented a second TfheRustBool Emitter pass, specifically for the packed API (--emit-tfhe-rust-bool-packed)
The former emitter pass (--emit-tfhe-rust-bool) still outputs boolean tfhe-rs ready code.

…s update

Adding the (common) enum attribute to model Boolean gates
Copy link
Collaborator

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thank you! I left a few initial comments, really great work on unifying the emitter and handling the elementwise cases as well for the packed API.

Could you please rebase the PR to main and then also add a test case for just emitting some packed operations (not end to end tests) just to test the expected syntax of the emitter, something like https://github.com/google/heir/blob/main/tests/tfhe_rust/emit_tfhe_rust.mlir?

lib/Dialect/TfheRustBool/IR/TfheRustBoolEnums.h Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
lib/Target/TfheRustBool/TfheRustBoolEmitter.cpp Outdated Show resolved Hide resolved
@WoutLegiest WoutLegiest force-pushed the dev-emit branch 3 times, most recently from cc32f0e to 039e9ab Compare September 3, 2024 12:51
…s update

Update the TRB Emitter with the packed API
Copy link
Collaborator

@asraa asraa left a comment

Choose a reason for hiding this comment

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

Thank you! Could you check if you can rebase this / squash into a single commit? (I think the first commit has already been merged)

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