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

SelectVariableNames: use suggested name from dialect #1225

Conversation

ZenithalHourlyRate
Copy link
Collaborator

This is inspired by #1219 and shares some code with #1219. This will generate more friendly backend code, for example, openfhe-pke emitter output becomes

CiphertextT dot_product(CryptoContextT cc, CiphertextT ct, CiphertextT ct1) {
  std::vector<int64_t> v = {0, 0, 0, 0, 0, 0, 0, 1};
  const auto& ct2 = cc->EvalMultNoRelin(ct, ct1);
  const auto& ct3 = cc->Relinearize(ct2);
  const auto& ct4 = cc->EvalRotate(ct3, 4);
  ...
}

CryptoContextT dot_product__generate_crypto_context() {
  CCParamsT params;
  params.SetMultiplicativeDepth(2);
  params.SetPlaintextModulus(4295294977);
  CryptoContextT cc = GenCryptoContext(params);
  cc->Enable(PKE);
  cc->Enable(KEYSWITCH);
  cc->Enable(LEVELEDSHE);
  return cc;
}

And all @heir//tests/Examples/openfhe/... pass with this change. However, other backend like Jaxite/TFHE-RS are relying on getIntForValue to properly function. I am not so familiar with them so mark as draft now.

@ZenithalHourlyRate ZenithalHourlyRate force-pushed the suggest-select-variable-name branch from a183d19 to de082c4 Compare December 21, 2024 13:44
@ZenithalHourlyRate ZenithalHourlyRate force-pushed the suggest-select-variable-name branch from de082c4 to 7956696 Compare January 7, 2025 19:16
@ZenithalHourlyRate
Copy link
Collaborator Author

However, other backend like Jaxite/TFHE-RS are relying on getIntForValue to properly function. I am not so familiar with them so mark as draft now.

Tweaked the selectVariableName process for these backend tests regarding the default prefix "v".
Namely these tests assume v0 for the first appearance then v1/v2, but for simplicity like for the openfhe backend, the crypto_context only appears once in one function so I intend to use cc instead of cc0 for it.

Ready for review now.

@ZenithalHourlyRate ZenithalHourlyRate marked this pull request as ready for review January 7, 2025 19:20
@ZenithalHourlyRate ZenithalHourlyRate force-pushed the suggest-select-variable-name branch from 7956696 to 9a4a95f Compare January 7, 2025 19:22
@j2kun j2kun added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Jan 7, 2025
@copybara-service copybara-service bot merged commit 3b91726 into google:main Jan 9, 2025
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants