-
Notifications
You must be signed in to change notification settings - Fork 111
Build-system: adding support for Apple Accelerate library #2707
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
Conversation
687f7fe to
14d9a14
Compare
|
@lucbv Could you rebase on develop? |
d9497ad to
6cb435a
Compare
|
Rebased |
brian-kelley
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aside from @JBludau's comment everything looks good to me!
|
Okay, I cleaned the CMake logic a bit and as a side effect, the ci build with accelerate is also a bit cleaner now. This should be good to go at this point : ) |
JBludau
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CMake part looks fine. Not sure if my other comment is relevant
| /* */ std::complex<float>* x, const KK_INT* x_inc); | ||
| void F77_BLAS_MANGLE(zscal, ZSCAL)(const KK_INT* N, const std::complex<double>* alpha, | ||
| /* */ std::complex<double>* x, const KK_INT* x_inc); | ||
| } // extern "C" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm that looks weird since it is touching the #else branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is fully contained in the "else" and the branch is based on which library is being used since they are written in different languages...
Adding option to request the library and modifying the BLAS logic to skip the Fortran check and correctly define the mangling macro for Accelerate. This last part might be an undue overload of the macro purpose? Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
This will allow us to make sure the logic we are adding it indeed working correctly and tested with the regular CI actions. Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Actually including the proper header... renaming a bit the github action to make it clear which one tests Accelerate. Adding CXX flag to force using the new version of Accelerate and avoid deprecation warnings. Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Further refactoring would be good in the future to centralize the various BLAS/CBLAS flavors and have them marshalled at configure time in a way that let's use less macros for all these different implementations that end up in the same Kokkos Kernels wrappers. Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Now the compiler and linker flags are specified using our cmake functions/macros which leads to cleaner code and hopefully leads to a good example for future reference. Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
After fixing the CMake logic, the osx-ci build can be cleaned up, we no longer need to pass the linker flag manually. Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
Some interfaces used very confusing/suspicious names for vectores and flags. This should look more standard and reasonable now! Signed-off-by: Luc Berger-Vergiat <lberge@sandia.gov>
ee6414b to
22ddff3
Compare
Adding option to request the library and modifying the BLAS logic to skip the Fortran check and correctly define the mangling macro for Accelerate. This last part might be an undue overload of the macro purpose?