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

Add Secret dialect #78

Closed
wants to merge 1 commit into from
Closed

Add Secret dialect #78

wants to merge 1 commit into from

Conversation

j2kun
Copy link
Collaborator

@j2kun j2kun commented Jul 31, 2023

This PR implements a secret dialect to represent generic, high level computation on encrypted data, along with a demonstration lowering pass that drops the secret types.

It defines a single new type, secret.secret<AnyType> which wraps a non-secret type, and centers around the secret.generic op, which "lifts" a computation on the plaintext types to a computation on the corresponding secret types, with some metadata attached to the high level operation encapsulated by a secret.generic so that it can be lowered to custom ops in specific dialects.

I also defined a pass, --secret-forget-secrets, which simply drops the secrets from all types and returns an equivalent program that operates on plaintext data.

Some notes:

  • In my examples I have secret.generic's body containing a single op, but I think more realistically we will have front ends that convert large regions of code into a large secret.generic, and then passes in this dialect that transform it by doing things like extracting loops with static bounds outside of the body of a secret.generic and only having the inner-most loop body contain secret.generic ops (which could then be further analyzed/optimized with partial loop unrolling and fusing), or something like transforming the body of a secret.generic into a combinational circuit, then applying a circuit optimizer to it.
  • Forgetting secrets is only intended to be useful for testing, e.g, to ensure a program is functionally equivalent before and after any FHE passes are applied. But it also helps me understand how to write passes that operate on the mildly complex IR I've defined for secret.generic.
  • I'd like to eventually generalize the forget-secrets pass to operate on an interface(s), and then apply that interface to lower level dialects, so that at any point during a program's compilation lifecycle, we can exit the "encrypted" part of the pipeline and test functional equivalence.

@j2kun j2kun force-pushed the secret branch 6 times, most recently from 07800c9 to 18f59ba Compare August 3, 2023 23:08
@j2kun
Copy link
Collaborator Author

j2kun commented Aug 3, 2023

Note to self: check out llvm-project/mlir/lib/Dialect/Linalg/Transforms/Detensorize.cpp as a guiding pass, and use the DialectConversion framework to make the secret type illegal.

@j2kun j2kun force-pushed the secret branch 2 times, most recently from bc33106 to 1fe91b4 Compare August 8, 2023 16:31
@j2kun j2kun marked this pull request as draft August 8, 2023 23:48
@j2kun j2kun force-pushed the secret branch 5 times, most recently from 48eddb9 to a1b2d4f Compare August 9, 2023 23:58
@j2kun
Copy link
Collaborator Author

j2kun commented Aug 10, 2023

There are a handful of TODOs still littered throughout the PR, for improved documentation and my confusion about the dialect conversion framework. But I think now is a good time for an initial review by @asraa and @AlexanderViand-Intel

@j2kun j2kun marked this pull request as ready for review August 10, 2023 00:24
@j2kun j2kun marked this pull request as draft August 15, 2023 15:44
@j2kun j2kun marked this pull request as ready for review August 16, 2023 21:13
@j2kun j2kun force-pushed the secret branch 2 times, most recently from e6c1876 to a2d7beb Compare August 16, 2023 21:46
include/Dialect/Secret/IR/BUILD Show resolved Hide resolved
include/Dialect/Secret/IR/SecretOps.td Show resolved Hide resolved
include/Dialect/Secret/IR/SecretOps.td Outdated Show resolved Hide resolved
include/Dialect/Secret/IR/SecretOps.td Show resolved Hide resolved
@j2kun
Copy link
Collaborator Author

j2kun commented Aug 17, 2023

I think there is some benefit to having this checked in even if we think doing it in full generality is too ambitious. It will be useful for some experimentation and @asraa and I are interested in trying it out as an entry point for some ML-specific dialects.

This dialect adds secret.secret<T>, a generic wrapper for secret types,
along with three basic ops, secret.conceal/reveal for generic
encrypt/decrypt ops (and dialect conversion materialization), as well as
a secret.generic op that lifts a plaintext computation to operate on
secrets, as an interface layer to the rest of the MLIR ecosystem.

Found a workaround for upstream issue https://reviews.llvm.org/D158107

The workaround is that using ConversionPatternRerwiter::replaceOp does
not exercise the same stack trace, whereas combining replaceAllUsesWith
+ eraseOp does.
Copy link
Collaborator

@AlexanderViand-Intel AlexanderViand-Intel left a comment

Choose a reason for hiding this comment

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

Only minor comments

include/Dialect/Secret/IR/SecretOps.td Show resolved Hide resolved

include "mlir/Pass/PassBase.td"

def SecretForgetSecrets : Pass<"secret-forget-secrets"> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Forgetting" secrets to me sounds like something that removes secret values, rather than the secret annotation. How about naming this "drop-secret-status" or something along those lines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking of the "forgetful functor" but maybe that is too big of a stretch.

I like the slightly more specific "drop-secret-to-cleartext" or "interpret-secret-as-cleartext", will update it on Monday.

@j2kun
Copy link
Collaborator Author

j2kun commented Aug 21, 2023

Something got messed up with the internal integration of this PR, so I'm going to cut an internal patch which will show up as a PR with some weird changes in it (including the changes for Alex's two comments)

copybara-service bot pushed a commit that referenced this pull request Aug 21, 2023
…namespaces

were deleted by the cleanups, which was marked as irreversible. I just removed
them for simplicity.

Also add two changes in response to @AlexanderViand-Intel's comments on
#78

FUTURE_COPYBARA_INTEGRATE_REVIEW=#78 from j2kun:secret cb8fa6a
PiperOrigin-RevId: 558818663
copybara-service bot pushed a commit that referenced this pull request Aug 21, 2023
…namespaces

were deleted by the cleanups, which was marked as irreversible. I just removed
them for simplicity.

Also add two changes in response to @AlexanderViand-Intel's comments on
#78

FUTURE_COPYBARA_INTEGRATE_REVIEW=#78 from j2kun:secret cb8fa6a
PiperOrigin-RevId: 558818663
j2kun pushed a commit that referenced this pull request Aug 21, 2023
COPYBARA_INTEGRATE_REVIEW=#78 from j2kun:secret cb8fa6a
PiperOrigin-RevId: 558845853
@j2kun
Copy link
Collaborator Author

j2kun commented Aug 21, 2023

Manually merged as 69db9f0

@j2kun j2kun closed this Aug 21, 2023
@j2kun j2kun deleted the secret branch December 12, 2023 06:00
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.

None yet

3 participants