Skip to content

make tfhe-rs a bazel dependency #1743

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

asraa
Copy link
Collaborator

@asraa asraa commented Apr 18, 2025

  • Adds a tfhe_rs_lib bazel macro that runs heir-opt / heir-translate / rust_library
  • Updates all CPU tests to an end to end bazel macro

FPGA tests:
Left the Cargo.toml with the local tfhe-rs folder, and the RUN commands as documentation.

Fixes #235

@asraa
Copy link
Collaborator Author

asraa commented Apr 18, 2025

Oops. Now the old cargo | Filecheck tests are giving me this error:

# RUN: at line 2
cargo run --release --manifest-path /usr/local/google/home/asraa/.cache/bazel/_bazel_asraa/dd6a35b586d0a2d65a5a8939e7a649cf/execroot/_main/bazel-out/k8-opt/bin/tests/Examples/tfhe_rust/add_one.mlir.test.runfiles/_main/tests/Examples/tfhe_rust/Cargo.toml --bin main_add_one -- 2 --message_bits=3 | FileCheck /usr/local/google/home/asraa/.cache/bazel/_bazel_asraa/dd6a35b586d0a2d65a5a8939e7a649cf/execroot/_main/bazel-out/k8-opt/bin/tests/Examples/tfhe_rust/add_one.mlir.test.runfiles/_main/tests/Examples/tfhe_rust/add_one.mlir
# executed command: cargo run --release --manifest-path /usr/local/google/home/asraa/.cache/bazel/_bazel_asraa/dd6a35b586d0a2d65a5a8939e7a649cf/execroot/_main/bazel-out/k8-opt/bin/tests/Examples/tfhe_rust/add_one.mlir.test.runfiles/_main/tests/Examples/tfhe_rust/Cargo.toml --bin main_add_one -- 2 --message_bits=3
# .---command stderr------------
# | 'cargo': command not found
# `-----------------------------
# error: command failed with exit status: 127

--

********************
********************
Failed Tests (1):
  heir :: Examples/tfhe_rust/add_one.mlir


Testing Time: 0.05s

Total Discovered Tests: 1
  Failed: 1 (100.00%)

@asraa asraa changed the title poc: add tfhe-rs with bazel deps make tfhe-rs a bazel dependency Apr 23, 2025
@asraa asraa requested a review from WoutLegiest April 23, 2025 20:59
@WoutLegiest
Copy link
Collaborator

Yeah, for the FPGA tests I run them locally on the FPGA server (so really a copy past approach). Normally, the FPGA code in HEIR should represent the small testbench that we can use it from.
Although, we should update it a bit more with all of the new tfhe-rs-belfort libraries and such.

@asraa asraa force-pushed the tfhe-rs-bazel branch 3 times, most recently from 9547d3d to 38ce0b7 Compare April 24, 2025 16:20
@j2kun
Copy link
Collaborator

j2kun commented Apr 24, 2025

Go ahead and delete

# Tests specifically for the tfhe-rs codegen
- name: rustup toolchain install
uses: dtolnay/rust-toolchain@439cf607258077187679211f12aa6f19af4a0af7 # [email protected]
with:
toolchain: stable
- name: Test rust codegen targets
run: |
bash .github/workflows/run_rust_tests.sh
and https://github.com/google/heir/blob/main/.github/workflows/run_rust_tests.sh to get the CI to pass, since it's only failing because there are no more targets to test.

@asraa asraa requested a review from j2kun April 24, 2025 17:37
Copy link
Collaborator

@j2kun j2kun left a comment

Choose a reason for hiding this comment

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

Nicely done! Just a few nits, hopefully implying more deletion

Copy link
Collaborator

@WoutLegiest WoutLegiest left a comment

Choose a reason for hiding this comment

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

Cool! Thanks
Maybe something we should discuss during one of the meetings is how we see the FPGA testings and if we even want the run results (since the outputted code is the same as CPU code)

@WoutLegiest WoutLegiest added the squash ready The PR is ready to be squashed into a single commit! label Apr 25, 2025
@asraa asraa added the pull_ready Indicates whether a PR is ready to pull. The copybara worker will import for internal testing label Apr 25, 2025
@asraa asraa force-pushed the tfhe-rs-bazel branch 2 times, most recently from 96d6c4a to 7642df8 Compare April 25, 2025 14:51
Signed-off-by: Asra Ali <[email protected]>

fix sbox timeout

Signed-off-by: Asra Ali <[email protected]>

remove test files

Signed-off-by: Asra Ali <[email protected]>

address comments

Signed-off-by: Asra Ali <[email protected]>

remove unneeded deps

Signed-off-by: Asra Ali <[email protected]>
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 squash ready The PR is ready to be squashed into a single commit!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate tfhe_rust e2e tests to use bazel rust rules.
3 participants