Draft: Fix cmake threads

1. Store CMAKE_THREAD_LIBS_INIT in the cache file for integration with ICON.
2. Fix the -pthread problem when building with NAG.
3. A bit of clean-up and consistency.

requested review from @b381660

assigned to @b381001

assigned to @m300488 and unassigned @b381001

@b381001 I will fix the pipeline and check the ICON integration tomorrow.

Ok thank you. This MR partly replaces !21 (closed). You can push to this branch in ICON : https://gitlab.dkrz.de/icon/icon-mpim/-/merge_requests/213

added 2 commits

• 74f8215a - Support building with the NAG compiler
• d36fb474 - Targeted header and module search path flags

Compare with previous version

mentioned in issue #7

LGTM. I mostly have minor remarks.

added 1 commit

• 7269aa6c - Collapse the generator expressions

Compare with previous version

added 3 commits

• 4837f295 - Support building with the NAG compiler
• a0bd8b31 - Targeted header and module search path flags
• c87b33d1 - Collapse the generator expressions

Compare with previous version

I might be the wrong place to discuss this but the question is related to how we build the library. The question is whether we need the util_backtrace functionality, which now has the mandatory Threads dependency, for all Fortran compilers. Don't some of them already have something similar, which gets enabled with some extra compiler flags? If so, should we still build util_backtrace for those compilers?

Threads is only used when found on a platform, otherwise util_backtrace compiles w/o it. So it is an optional dependency. But could be that cmake-scripting is not accurate here.

I did quite some research about different backtrace option. I.e. intel provides a traceback-function, but it is crap. The output only coniste of some addres in hex-format.

For NEC-compiler there is indeed a built-in function to call. For this case I used it. See https://gitlab.dkrz.de/icon-libraries/libfortran-support/-/blob/master/src/util_backtrace.c?ref_type=heads#L364

For nvhpc, gcc and nag I could not find suitable functions.

A clean traceback is a requirement for many developper. That's why people "improved" it by raising seg-faults or compiling incorrect OpenACC code to trigger it.

I really think pthread offers substantial improvement here.

Thanks for the info! I will update the CMake scripts then to reflect the fact that it's possible to build without Threads. You will have to resolve one issue though: if I understand correctly, these functions are declared in execinfo.h, which might be not available on a system (we even check for this but it looks like we don't use the result of the check, i.e. HAVE_EXECINFO_H is not used anywhere).

Implemented in !24 (merged)

Please merge if you are happy with the solution. Should be identical to what has been in place in ICON before fortran-support.

FYI: I am off tomorrow and entire next week.

@b381660 this is misleading, isn't it? I mean if it's a cache variable, the user might want to use it to override the result of find_package(Threads), which does not happen at the moment. Should we, maybe, generate a separate text file instead with the value of the variable? This is all hacky and dark. I need ideas on this as we kind of trying to establish a good example here.

That's a valid concern, but in general, I don't think the user should overwrite this value, which in fact appears to be the core of the FindThreads logic. The user would need to either not use this, or change the properties of Threads::Threads directly instead, see https://github.com/Kitware/CMake/blob/master/Modules/FindThreads.cmake#L238

A new file would make us lose the easy-to-use set() command AFAIK. If we still want to avoid clashes, do we need to use the same variable name? My understanding is you are going to use this storage file to search for this on the ICON/autoconf side, right?

added 2 commits

• e56078da - cfallback if execinfo.h not available
• 9a2c13c5 - Merge branch 'check_execinfo' into 'fix-cmake-threads'

Compare with previous version

added 10 commits

• 9a2c13c5...b3d5a699 - 2 commits from branch master
• 1c546a8b - Refer to the target consistently
• f111da83 - Move find_package to the root CMakeLists.txt
• a30b97bb - Store CMAKE_THREAD_LIBS_INIT in the cache file (for ICON)
• 99700759 - Do not check for pthread.h twice
• e5ed0566 - Support building with the NAG compiler
• 1e4b0fdd - Targeted header and module search path flags
• aa7307cc - Collapse the generator expressions
• 4bb356cc - cfallback if execinfo.h not available

Compare with previous version

I looked at the implementation of util_backtrace and found that it is sufficiently broken. For example, file->offset + frame->addr (here) makes no sense. It works on Levante by coincidence only because the compilers there are configured to generate non-position-independent executables (i.e. -no-pie) by default and the executable "section" is mapped with zero offset. None of that is something to rely on.

What is your way forward then?

For me this solution is good enough since it produces good results for almost all BB-builders, eventough it is simply by chance.

I should have checked this earlier: the dependency on Threads is redundant.

closed