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

Use mutable/in-place operations in OpenFhePkeEmitter where possible #1327

Open
AlexanderViand-Intel opened this issue Jan 28, 2025 · 3 comments
Labels
dialect: openfhe Issues related to the openfhe dialect

Comments

@AlexanderViand-Intel
Copy link
Collaborator

AlexanderViand-Intel commented Jan 28, 2025

In addition to the eval<...> functions, OpenFHE also provides eval<...>InPlace which overrides one of the operands,
There's also eval<...>Mutable where it will do any necessary preprocessing (e.g., rescaling/etc) of the operands in-place,
and eval<...>MutableInPlace, where it will do both the preprocessing and the operation in-place.

The plain eval<...> always forces a copy, eval<...>InPlace can still force a copy if pre-processing is necessary (as it's only allowed to modify/overwrite one operand), while eval<...>MutableInPlace will never cause copies. Since we know if an SSA value has other uses*, we should be able to determine fairly easily in printEvalMethod if a given op can be lowered to one of the more efficient versions.

For small "toy" programs, the performance differences are very much negligible, but for larger real-world workloads, memory tends to become a significant bottleneck, so doing this "correctly" should be beneficial. Filing this as an issue, rather than putting together a PR, as it's just not very critical at the moment 😉 |

*Thinking about this again, hasOneUse() is correct but too conservative, as we don't care about prior uses. But MLIR's Liveness analysis has isDeadAfter which sounds like exactly what we want.

@AlexanderViand-Intel AlexanderViand-Intel added the dialect: openfhe Issues related to the openfhe dialect label Jan 28, 2025
@asraa
Copy link
Collaborator

asraa commented Jan 28, 2025

+1000! I've wanted to do this, and didn't know about isDeadAfter - which is such a cool feature.

If for some reason someone ever writes an interpreter, that would also be a really useful function for it. For e.g. jaxite IR in a python interpreter @code-perspective

You may see me tackle this at the end of the week for demo speed purposes, but I will assign myself / update the issue when I do.

@ZenithalHourlyRate
Copy link
Collaborator

I thought about those Inplace optimizations as well (Lattigo prefers in-place API). I was thinking about converting value semantic to memref semantic at some, maybe lwe, stage, then use canonicalization rule to rewrite copy op into inplace op.

I think converting to memref semantic is needed to correctly model the memory effect behavior.

@AlexanderViand-Intel
Copy link
Collaborator Author

AlexanderViand-Intel commented Jan 29, 2025

I think converting to memref semantic is needed to correctly model the memory effect behavior.

That might be overkill for this specific issue - even a "conservative" estimate using Liveness should give some pretty good results. But then again we will eventually need to consider bufferization for non-llvm targets anyway, and doing it on the lwe.ctxt/lwe.ptxt level would mean no duplication.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dialect: openfhe Issues related to the openfhe dialect
Projects
None yet
Development

No branches or pull requests

3 participants