-
Notifications
You must be signed in to change notification settings - Fork 57
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
openfhe: add bootstrap op #1195
openfhe: add bootstrap op #1195
Conversation
a76d360
to
885b3f3
Compare
885b3f3
to
42d961e
Compare
Rebased to new lwe type. Tests will be added later. |
Are there any downsides to building OpenFHE with OpenMP and a fast math backend (apart from adding additional dependencies, I guess?). If no, then we should consider doing so as the internal default, especially if people indeed end up using HEIR for benchmarking.
In general, I think it's fine to add low-level operations that the high-level parts of the compiler don't know how to generate (a lot better than the reverse, at least). However, I feel like it'd be nice to also add |
42d961e
to
cbf1c02
Compare
Definitely. Now the e2e test is added. Check
I do agree we should integrate a fastest openfhe for proper evaluation, but that involves a lot of bazel (possibly needs jeremy). Also, github actions CI is not super powerful so it won't benefit much from it. Instead I choose to use a smaller parameter for the testing pipeline and added a warning on the test on the insecurity for other people wanting to use it. |
When I first set this up, the obstacle was the cmake indirection layer. I will file an issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
bool result = false; | ||
op.walk<WalkOrder::PreOrder>([&](Operation *op) { | ||
if (isa<openfhe::BootstrapOp>(op)) { | ||
result = true; | ||
return WalkResult::interrupt(); | ||
} | ||
return WalkResult::advance(); | ||
}); | ||
return result; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I will clean these up after this PR goes in, since we have a few implementations of "walk until you find fn(op)" in the codebase.
There are some merge conflicts. If you don't have time to fix them I can do an internal patch tomorrow to resolve it. |
cbf1c02
to
a3c5ce8
Compare
Rebased. |
a3c5ce8
to
c7e338e
Compare
c7e338e
to
9ddcafb
Compare
This follows the setup step in simple-ckks-bootstrapping example in OpenFHE.
With input
with
--openfhe-configure-crypto-context
we getThen the translated code can run with a main function similar to
simple-ckks-bootstrap
.To make it run faster, the following setup step is needed
Link the generated code against an OpenFHE with multi-thread support and fast math backend (HEIR internal one is single-thread, slow math backend). It could run within 3 second (96 core machine). Link against HEIR internal one would require about 2 minutes to run.
Discussion
mgmt.bootstrap
op and implement a bootstrap management pass (possibly related to Investigate what parts of DaCapo can be ported to HEIR #289, Investigate if/how to integrate Saturn into HEIR #1166).bootstrapDepth
(level consumed by bootstrap) is hard-encoded with value taken from openfhe. This depth will affect logN selection hence slot num, which is needed by theEvalBootstrapKeyGen
. Now I acquire this time in runtime (by the emitter) fromcryptoContext
instead of putting it inopenfhe.gen_bootstrap_key
op.