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

Extend blt_check_code_compiles macro to handle dependencies #639

Conversation

mcfadden8
Copy link
Contributor

@mcfadden8 mcfadden8 commented May 25, 2023

While developing LLNL/Umpire#807, we needed to be able to detemine whether there was HIP suppport for course grained memory coherence. We wanted to attempt to compile segments of code that used this functionality to determine compile-time support.

@mcfadden8 mcfadden8 requested a review from white238 May 25, 2023 23:24
@mcfadden8 mcfadden8 changed the title Add LINK_LIBRARIES parameter to blt_check_code_compiles macro Extend blt_check_code_compiles macro to handle dependencies May 25, 2023
Copy link
Member

@kennyweiss kennyweiss left a comment

Choose a reason for hiding this comment

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

Thanks @mcfadden8

Is there a simple test that could exercise this new option?
We have one for blt_check_code_compiles (without this option), here:

message(STATUS "Checking for blt_check_code_compiles")
blt_check_code_compiles(CODE_COMPILES _hello_world_compiled
VERBOSE_OUTPUT ON
SOURCE_STRING
[=[
#include <iostream>
int main(int, char**)
{
std::cout << "Hello World!" << std::endl;
return 0;
}
]=])
if(_hello_world_compiled)
message(STATUS "... passed")
else()
message(FATAL_ERROR "... failed to compile.")
endif()

@mcfadden8
Copy link
Contributor Author

@kennyweiss and @white238, I was thinking that a good way to test this would be to find a library that we would need to FIND_PACKAGE on and then pass that library in to blt_check_code_compiles. Besides major things like cuda and hip, I have not yet found a package that fits the bill. I tried zlib and sqlite3, but both of those packages seem to have been installed on the system already.

I was thinking I could try a test like:

Test 1. if (FIND_PACKATE(x)) - Expect blt_check_code_compiles(src, x) to succeed
Test 2. else - Expect blt_check_code_compiles(src) to fail

Can anyone think of good library candidates to use or perhaps a different way of testing it?

One option is that I could just use FIND_PACKAGE(sqlite3) and use it. It would exercise test 1 of the code, but not necessarily test 2 until the test was in an evironment that did not have sqlite3 available.

Am I overthinking this? Should I be satisfied that test 1 is sufficient?

@white238
Copy link
Member

I think that adding a guarded test for MPI then just running a duplicate of the MPI smoke test should be sufficient.

But you could also add a hip/cuda one.

@rhornung67
Copy link
Member

@mcfadden8 what about camp?

@mcfadden8
Copy link
Contributor Author

Tests added. Please let me know if you have any additional adjustments I should make. Thanks

RELEASE-NOTES.md Outdated Show resolved Hide resolved
mcfadden8 and others added 2 commits May 26, 2023 14:57
Co-authored-by: Chris White <white238@llnl.gov>
Co-authored-by: Chris White <white238@llnl.gov>
cmake/BLTMacros.cmake Outdated Show resolved Hide resolved
Also, change `LINK_LIBRARIES` to be 'DEPENDS_ON` for internal
consistency.
@white238 white238 merged commit e694578 into develop Jun 6, 2023
8 checks passed
@white238 white238 deleted the feature/mcfadden8/add_link_libraries_to_blt_check_code_compiles branch June 6, 2023 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants