-
Notifications
You must be signed in to change notification settings - Fork 120
Add extended basis rotation operations for OpenFHE dialect #2574
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
base: main
Are you sure you want to change the base?
Add extended basis rotation operations for OpenFHE dialect #2574
Conversation
- Add FastRotationExtOp, KeySwitchDownOp, MulExtOp, AddExtOp, AddExtInPlaceOp
|
I'd appreciate feedback on the operation signatures in OpenfheOps.td (are the arguments and attributes appropriate?), whether we need a distinct type for extended-basis ciphertexts or can reuse Openfhe_Ciphertext, and any concerns with the overall approach before I proceed with implementing the conversion pass and optimization patterns to push |
asraa
left a comment
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.
Thank you so much! Great job on starting the tablegen and making a small PR
| ["isBatchCompatible", "buildBatchedOperation"]> | ||
| ]> { | ||
| let summary = "Fast (hoisted) rotation in extended (P*Q) CRT basis."; | ||
| let arguments = (ins |
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.
It looks like from https://github.com/openfheorg/openfhe-development/blob/aa391988d354d4360f390f223a90e0d1b98839d7/src/pke/include/cryptocontext.h#L2438
that the arguments needed are (cryptoContext, input, index, precomputedDigitDecomp, addFirst) - in other words i don't think you neeed cyclotomicOrder anymore.
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.
+1, I submitted the patch to remove that argument, though we should double check it's in the OpenFHE version we're depending on
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.
your patch was only for the regular evalfastrotation, this API was unchanged and i guess never had that m param
| MLIRContext* context, OpBuilder& builder, | ||
| SmallVector<Value> vectorizedOperands, | ||
| SmallVector<Operation*> batchedOperations) { | ||
| SmallVector<Value> indexVals; |
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 wonder if you can share a function with EvalFastRotation? The code is very very similar.
Summary
This PR adds support for extended basis operations in the OpenFHE dialect, allowing the use of an optimization technique where multiple rotations are performed in a larger (P*Q) CRT basis and combined before a single
key_switch_downconverts the result back to the normal (Q) basis.This is groundwork for addressing #2519.
New Operations
openfhe.fast_rotation_extopenfhe.key_switch_downopenfhe.add_extopenfhe.add_ext_inplaceopenfhe.mul_extEvalMultNoRelin)