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.

Sign up for GitHub

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

Jump to bottom

Enable fft_small module for CMake builds #1629

Merged
merged 41 commits into from
Nov 21, 2023
Merged

Enable fft_small module for CMake builds #1629

merged 41 commits into from
Nov 21, 2023

Conversation

albinahlback
Copy link
Collaborator

@albinahlback albinahlback commented Nov 17, 2023 •
edited
Loading

I made the fft_small module available for MSVC systems that supports AVX2 or ARM NEON instructions.

Also changed definitions of FLINT_NORETURN and FLINT_TLS_PREFIX to the C standards.

I also enabled test checking for the Ubuntu CMake runner.

@albinahlback albinahlback force-pushed the fft_small_cmake branch 3 times, most recently from cdb4309 to 9dca714 Compare November 18, 2023 11:40
@albinahlback albinahlback force-pushed the fft_small_cmake branch 2 times, most recently from 7d088dd to d009ed9 Compare November 18, 2023 13:30
@albinahlback albinahlback mentioned this pull request Nov 18, 2023
Closed
@albinahlback
Copy link
Collaborator Author

Okay, I think I found the error! It seems to be the non-GNU version of _madd in crt_helpers.h.

@albinahlback
Fix fft_small printing in test
39d4ccd
@albinahlback
fix _madd in crt_helpers.h
1dbf93a
@albinahlback
Remove extern from function declaration in qadic test
cba58ce
@albinahlback
Enable fft_small module for CMake builds
6ae542d
@albinahlback
Make machine_vectors.h usable for MSVC
764482e
@albinahlback
Make crt_helpers.h usable for MSVC
e27a02a
@albinahlback
Replace sizes for arrays in fft_small/mpn_mul.c
3a117f3 
Used to be variable expressions, now changed to constant expressions
@albinahlback
Remove TEMPLATE_DIRS from CMake
28ab84f
@albinahlback
Fix FLINT_WANT_ASSERT in fft_small
f53981f
@albinahlback
Fix MSVC warnings
774fee8
@albinahlback
Cleanup CMake config.h.in
2a18b47
@albinahlback
Set FLINT_NORETURN according to C standards
8416820
@albinahlback
Set FLINT_TLS_PREFIX according to C standards
59701e6
@albinahlback
Disable fft_small for MSVC
ff8b8a4 
And print fft_small module check for CMake
@albinahlback
Run tests for CMake Ubuntu runner
34e5e95
@albinahlback
Fix definitions of FLINT_NORETURN and FLINT_TLS_PREFIX for C++
b12075b
@albinahlback
Fix C++ in configure.ac if building NTL tests
f4283a5
@albinahlback
Disable checking if TLS is available during configuration
716f3b8 
We use C11, so it should be supported
@albinahlback
Remove redundant section on FLINT_DLL in code_conventions.txt
328ef28
@albinahlback
Fix types of length instances test_helpers.h main function
277e30d
@albinahlback
Merge branch 'fft_small_cmake' into msvc_fft_small
aa29777
@albinahlback
Fix assertion in qadic for Windows
f6f52e9
@albinahlback
Copy link
Collaborator Author

Hopefully this fixes #1629

@albinahlback
Copy link
Collaborator Author

Hey, it does work! Summoning @fredrik-johansson!

I messed up with the merge of two different branches, so I never managed to squash the commits properly and the commits are a little bit in the wrong order (for example, "disable fft_small for MSVC" comes after the enabling of it, which is incorrect). Hopefully this is okay though.

@fredrik-johansson
Copy link
Collaborator

Nothing really objectionable as far as I can tell.

The add_sssssaaaaaaaaaa etc. fallbacks look like they might not necessarily generate optimal code, so I'd at least like to have an issue open about profiling/improving that.

@albinahlback
Copy link
Collaborator Author

The add_sssssaaaaaaaaaa etc. fallbacks look like they might not necessarily generate optimal code, so I'd at least like to have an issue open about profiling/improving that.

Yes. X86 Windows could be using _addcarry_u64, but I'm unsure if ARM Windows have any better alternative. But let's open an issue about that instead.

@albinahlback albinahlback merged commit 9e8cfca into flintlib:trunk Nov 21, 2023
18 checks passed
@albinahlback albinahlback deleted the fft_small_cmake branch January 23, 2024 10:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fredrik-johansson fredrik-johansson Awaiting requested review from fredrik-johansson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@albinahlback @fredrik-johansson