AI port of binfhe backend from ragahv/heir #2076
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements a complete BinFHE backend for OpenFHE, porting functionality from a previous implementation. It adds support for converting CGGI dialect operations to OpenFHE BinFHE operations and generates C++ code using the OpenFHE BinFHE API for boolean circuit evaluation.
- Adds CGGI to OpenFHE conversion pass with BinFHE-specific operations and types
- Implements OpenFHE BinFHE code emission and header generation
- Provides comprehensive end-to-end testing with logic operations (XOR, AND, OR, NOT) and full adder functions
Reviewed Changes
Copilot reviewed 39 out of 39 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/heir-translate.cpp | Registers new BinFHE translation functions |
| tools/heir-opt.cpp | Adds CGGI to OpenFHE conversion pass registration |
| tools/heir-openfhe-binfhe.bzl | Defines build macros for BinFHE library generation |
| lib/Target/OpenFhePke/OpenFheUtils.cpp | Extends type conversion for BinFHE types |
| lib/Target/OpenFhePke/OpenFheBinEmitter.cpp | Implements C++ code generation for BinFHE |
| lib/Dialect/Openfhe/IR/OpenfheTypes.td | Adds BinFHE-specific types (context, scheme, LUT) |
| lib/Dialect/Openfhe/IR/OpenfheOps.td | Defines BinFHE operations (LUT creation, evaluation, arithmetic) |
| lib/Conversion/CGGIToOpenfhe/CGGIToOpenfhe.cpp | Converts CGGI operations to OpenFHE BinFHE operations |
| tests/Examples/openfhe/binfhe/* | Comprehensive test suite for BinFHE functionality |
| "embedded", | ||
| "Emit OpenFHE with embedded import paths (default " | ||
| "for code to be included in OpenFHE source files)"))}; | ||
| "for HEIR-internal development)"))}; |
There was a problem hiding this comment.
The removal of the EMBEDDED enum value appears incomplete. The original code had two enum values but only SOURCE_RELATIVE remains, which may break existing code that uses the EMBEDDED option.
There was a problem hiding this comment.
Look at this - actually useful review comments from the 🤖 🫨 (this seems like an unnecessary/unrelated change that shouldn't be in this PR)
| } | ||
| template <class T, int dim> | ||
| std::vector<T> unflatten(std::vector<T> source, int start = 0) { |
There was a problem hiding this comment.
[nitpick] The template function 'unflatten' has inconsistent naming with other similar functions in the codebase. Consider using a more descriptive name like 'unflattenVector' or following the existing naming conventions.
| std::vector<T> unflatten(std::vector<T> source, int start = 0) { | |
| std::vector<T> unflattenVector(std::vector<T> source, int start = 0) { |
|
It seems like it will require quite a bit of cleanup, but go for it! |
2 participants
Maybe? solves #1238?
This is the result of me telling some AI system to please port @raghav198 's binFHE OpenFHE lowering/emitter to current HEIR and add some tests, and then yelling at the AI to "fix everything" until
bazel test //...passes. Not sure if it's worth wasting (human) review time on yet, I'll take a closer look at it myself next week but I wanted to record "my" progress here in the meantime.