configure: test linking with -fopenmp, test the resulting shared object #6642
+62
−63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Test loading the resulting shared objects; make sure they return an above-zero number of threads * Test -fopenmp on all Unix-like operating systems
* Make sure to include user-supplied PKG_{CFLAGS,LIBS}
* Be consistent about calling the linker flags LIBS, not LDFLAGS
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6642 +/- ##
=======================================
Coverage 98.61% 98.61%
=======================================
Files 79 79
Lines 14533 14558 +25
=======================================
+ Hits 14332 14357 +25
Misses 201 201 ☔ View full report in Codecov by Sentry. |
|
Generated via commit 4bd336f Download link for the artifact containing the test results: ↓ atime-results.zip
|
|
Works with my R4.2 installation on Mac. Doesn't work on my R4.4 or 4.3 installations (nothing I've tried so far has), still yields Full output attempting to install on R4.4.1
|
That's much fewer object files being re-compiled than linked into the final shared object. The danger and the usefulness of being able to run A comparison with the build output from R-4.2 might also help. |
Clean R4.2 build (successful)
Clean R4.4 build (failure)
|
R 4.2 config.log
R 4.4 config.log
|
It turns out it's possible for omp_get_num_threads() to succeed while failing to load the shared library that does more involved OpenMP operations.
|
Thank you very much for the test results!
Have you got two different arm64 Macs with different R versions on
them? Otherwise it's a bit surprising to see the same include and
library paths on the compiler command lines.
Is it a CRAN build of R or the Homebrew build of R?
Either way, the test we had so far failed to detect a broken
configuration, so let's try a more thorough test.
|
Nope, this is all on the same physical machine.
Pretty sure I installed all of these from the CRAN MacOS installers |
|
No dice with the new commit. Same error. Output and log:
|
|
Thanks again! Part of the problem may be that I'm currently out of ideas how to detect configurations like this one that seem to compile, link, load and perform OpenMP operations during |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dyn.load()the resulting shared object and run code from it before deciding to use these flags fordata.table-fopenmp, add it to both compiler and linker flagsHopefully, this will fix #6622 and prevent similar problems in the future. A more conservative fix is #6633.
Upsides:
data.tablewill install without OpenMP support-fopenmpis required in both compiler and linker flagsDownsides:
omp_get_max_threads()return a number ≤ 0 duringdata.tableinstallation, the build process will consider OpenMP not working and disable it.OMP_NUM_THREADS=0orOMP_THREAD_LIMIT=0results in warnings, butomp_get_max_threads()remains positive.-fopenmpis now tested unconditionally, not limited touname %in% c('Linux', 'Darwin'). Something that previously built single-threadeddata.tablemay now enable OpenMP and surprise someone.configurenow runs an R script. This can be rewritten back into a shell script if we can guaranteemktemp -davailable on everything that runsdata.table.