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

Basic ntt #109

Merged
merged 9 commits into from
Sep 12, 2024
Merged

Basic ntt #109

merged 9 commits into from
Sep 12, 2024

Conversation

hanno-becker
Copy link
Contributor

Moving #98 to main repo to enable benchmarking workflow.

@hanno-becker hanno-becker requested a review from a team September 8, 2024 05:27
@hanno-becker
Copy link
Contributor Author

@potsrevennil

I would like the configuration of OPT to be as simple as possible. As a user, I don't want to pass a complicated argument as NTT123_4567, but just say whether ASM should be enabled or not. Internally, or for more experienced users, we can also expose a fine-grained option, but I think this should be secondary.

Even when OPT is set, the AArch64 ASM should only be used when we are actually running on an AArch64 system. There should be a system header detecting this once and for all, exporting something like USE_AARCH64_ASM as the combination of running on an AArch64 system and OPT being set. Then, the actual source should cleanly rely on that flag only.

For benchmarking, I would suggest that we set OPT=1 by default, and add new -ref benchmarks which force OPT=0 and thereby fall-back to the reference implementation.

@mkannwischer You may want to chime in here.

@hanno-becker hanno-becker force-pushed the basic-ntt branch 2 times, most recently from af3096f to 807cf4e Compare September 9, 2024 08:47
@hanno-becker hanno-becker added the benchmark this PR should be benchmarked in CI label Sep 9, 2024
@hanno-becker hanno-becker force-pushed the basic-ntt branch 2 times, most recently from c062ced to d170147 Compare September 9, 2024 10:42
@hanno-becker hanno-becker added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Sep 9, 2024
potsrevennil and others added 6 commits September 12, 2024 05:12
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Signed-off-by: Thing-han, Lim <[email protected]>
Use OPT=AARCH64 at the toplevel to enable AArch64 optimizations.
Refined (internal) configuration options, e.g. for switching
between NTT implementations, to follow.

Signed-off-by: Hanno Becker <[email protected]>
@hanno-becker hanno-becker force-pushed the basic-ntt branch 4 times, most recently from deddf53 to 33eb2db Compare September 12, 2024 04:56
@hanno-becker hanno-becker added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Sep 12, 2024
@hanno-becker hanno-becker added benchmark this PR should be benchmarked in CI and removed benchmark this PR should be benchmarked in CI labels Sep 12, 2024
Signed-off-by: Thing-han, Lim <[email protected]>
@hanno-becker hanno-becker merged commit e5bc22a into main Sep 12, 2024
11 checks passed
@hanno-becker hanno-becker deleted the basic-ntt branch September 12, 2024 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
benchmark this PR should be benchmarked in CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants