Simplify -l cflags
#175
Simplify -l cflags
#175
Conversation
WalkthroughThe changes primarily revolve around the modification of build flags for different platforms across several files. The alterations include the addition and removal of certain flags, updating library paths, and specifying new directories. No changes to function signatures, global data structures, or interfaces have been made. Changes
## Walkthrough This pull request primarily focuses on modifying build flags across different platforms for various libraries such as HDF5, MPI, OpenBLAS, LAPACKE, and VMODROOT. The changes include removal, addition, and modification of flags, as well as updates to include and library paths. Changes
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (4)
- inout/h5/_cflags.c.v (1 hunks)
- mpi/_cflags.c.v (1 hunks)
- vlas/cflags_d_cblas.v (1 hunks)
- vlas/cflags_notd_cblas.v (1 hunks)
Files skipped from review due to trivial changes (4)
- inout/h5/_cflags.c.v
- mpi/_cflags.c.v
- vlas/cflags_d_cblas.v
- vlas/cflags_notd_cblas.v
|
Marking as draft as I'd like to re-check the changes for the blas related files. Feedback is already welcome. |
vlas/cflags_notd_cblas.v
Outdated
| #flag -I@VMODROOT | ||
| #flag -lopenblas -llapacke |
There was a problem hiding this comment.
we can remove -lopenblas from here 👌🏻
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Files selected for processing (1)
- vlas/cflags_notd_cblas.v (1 hunks)
Additional comments (Suppressed): 4
vlas/cflags_notd_cblas.v (4)
3-4: The
-llapackeflag has been removed from the Linux build flags. Ensure that this does not cause any linking issues with the LAPACKE library. If the library is still required, consider adding it back.6-6: The
-lopenblasflag has been removed from the Windows build flags. If OpenBLAS is still required, this could cause issues. Please verify.7-8: The include and library paths for the Darwin platform have been changed from
openblastolapack. If the OpenBLAS library is still required, this could cause issues. Please verify.10-10: The
-llapackeflag has been added for all platforms. Ensure that the LAPACKE library is available on all platforms and that this change does not cause any issues.
|
Marking as draft again to put some more time into it tomorrow or over the weeken. I think with further investigation I can start from here to make it a base to fix tests on mac. |
|
Just to add some context, the problem with vsl.vlas on MAC is that I was not able to use lapackE in Mac 🥲 |
|
Could do some more checks, the flags should be used correctly as they are now. Fixing the tests has it's own scope. I would fix LAPACKe usage and and tests on macOS in a followup PR and we could review and discuss there if that's okay. |
ttytm
force-pushed
the
refactor/simplify-cflags
branch
from
564649d to
55f8603
Compare
WalkthroughThe changes primarily revolve around the modification of build flags for different platforms across several files. The alterations include the addition and removal of certain flags, updating library paths, and specifying new directories. No changes to function signatures, global data structures, or interfaces have been made. Changes
## Walkthrough This pull request primarily focuses on modifying build flags across different platforms for various libraries such as HDF5, MPI, OpenBLAS, LAPACKE, and VMODROOT. The changes include removal, addition, and modification of flags, as well as updates to include and library paths. Changes
TipsChat with CodeRabbit Bot (
|
Summary by CodeRabbit
These changes aim to streamline the build process and enhance the software's performance across various platforms. Users may experience improved reliability and efficiency as a result.