Project

General

Profile

Task #2842

Task #2831: Bump required version numbers of infrastructure for 2020

Add libstdc++ check

Added by Roland Schulz 7 months ago. Updated 5 months ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Category:
-
Target version:
-
Difficulty:
uncategorized
Close

Description

We need a check in cmake that the libstdc++ version is sufficient (for clang and icc).
cmake doesn't seem to have a build in version check.
Since version 7 _GLIBCXX_RELEASE exists but not before.

Possible solutions:
- Use __GLIBCXX__. E.g. __GLIBCXX__==20150422 || (__GLIBCXX__>=20150716 && __GLIBCXX__!=20160803) (equivalent to ==5.1 or >=5.2 but not 4.9.4) see https://gcc.gnu.org/develop.html#timeline and https://gcc.gnu.org/onlinedocs/libstdc++/manual/abi.html#abi.versioning.__GLIBCXX__)
- Test whether features we (are planning to) use such as regex or make_unique work. But almost everything in C++14 work in libstdc++ 4.9. Do we want 4.9 to pass the test? Only added features in 5 are heterogeneous comparison lookup in associative container, and global cbegin, cend, rbegin, rend, crbegin, and crend (see https://www.gnu.org/software//gcc/gcc-5/changes.html). According to the link std::is_final is also missing in 4.9 but my tests can't confirm that.
- Test whether something only in 5 works e.g.

#include <iterator>
void f () { int a[2]; std::cbegin(a); }

- Test some undocumented define others are using. E.g. _GLIBCXX_REGEX_STATE_LIMIT is only available since 5.x and is suggested by https://stackoverflow.com/a/41186162/1385788

Associated revisions

Revision f232a741 (diff)
Added by Roland Schulz 6 months ago

Improve libstd++ handling

clang and icc use libstdc++ under Linux by default.

Previously such dependencies were handled differently than other
dependencies, in that we relied on the compiler autodetection, and
required the user to use the compiler flags to specify a proper
version.

Now we discover libstdc++ using cmake by finding g++, unless the user
or the toolchain they chose have already managed which gcc toolchain
to use. Then the found version is provided to the compiler with the
proper flags.

This has the advantages:
- Makes it easy for us to check the required version is used.
Version check for libstdc++ 5 for C++14 is added.
- Makes it easy for the user to provide the correct version
because standard cmake variables work.
- Makes build reproducible because the instance of libstdc++ used
is constant (e.g. cached via GMX_GPLUSPLUS_PATH, or controlled
by the toolchain, but not if the user chose non-reproducibility
by setting CXXFLAGS environment variables). Previously, the
auto-detection depended on the PATH environment variable at
the time of build (not the time of the cmake run). Thus
rebuilding with the same cache file was not guranteed to
give the same result. It could cause cause confusing link
errors if parts were rebuilt differently.

Also managed libc++ for clang-analyzer builds a bit better.

Fixes #2842

Change-Id: I855e16a6d4bd670cfb7acd6ea5c740f3a1b226bf

Revision a6b490a3 (diff)
Added by Mark Abraham 3 months ago

Provide better help for using libc++

This is now the actual cmake option required

Refs #2842

Change-Id: I1ab4676b0e01d65ba8df736b78eeb3c676cdc4ea

History

#1 Updated by Roland Schulz 7 months ago

  • Description updated (diff)

#2 Updated by Roland Schulz 7 months ago

  • Description updated (diff)

#3 Updated by Roland Schulz 7 months ago

Note: the reason we bumped the requirement for libstdc++ to 5.1 is partly for consistency to gcc (where we require 5.1 for relaxed constexpr support). If we would allow libstdc++ 4.9 it would only be allowed with clang/icc but not gcc.

#4 Updated by Mark Abraham 7 months ago

I could live with gcc 5.1 (plus its libstdc++), and icc/clang whatever+version-supports-c++14 (plus libstdc++-4.9) because that seems to work well enough. And we should have a feature check for the standard library, rather than depend on arcane not-known-stable macro tests.

But if we do that, then we need to have feature checks for the C++ standard library functionality that are very similar to those that we had until very recently.

@Erik, if you don't want those checks, then you have to come up with a better way of doing this. Important compilers (icc, clang) by design do/can depend on external c++ standard libraries that the user can choose. That's no different from any other library dependency, for which best practice is to test for feature support rather than version number. It's no use talking about five-year-old compilers, that's not the relevant question! :-)

#5 Updated by Roland Schulz 7 months ago

__GLIBCXX__ is officially supported way to check for version. So I don't think it falls into the "arcane not-known-stable macro tests" category. But being based on a date it has the weird property of not being monotonic when trying to check for version.
Why is it best practice to test for a feature rather than a version? And if so why does that not apply to the compiler and CUDA? In cases where there is a well documented and known version, than a version is in my opinion simpler and simpler is better. It also helps us to give a better error message because we can report the version and the version directly matches what we test for.

So if _GLIBCXX_RELEASE was supported starting with 5.x I would be strongly in favor of using that over feature test. But given the weirdness of __GLIBCXX__ I'm OK either way.

But even if we do a feature test, I'm strongly in favor of keeping it minimal rather than testing many/all C++ features we are using. Meaning I would only test for a single feature which was added in 4.9 or 5.1 (depending on which version we want to depend on). For 4.9 I would only test for regex being correct or make_unique being available:

#include <memory>
auto i = std::make_unique<int>();

For 5.1 I would only test e.g. cbegin.

#6 Updated by Roland Schulz 7 months ago

I just thought of a different solution which IMO is much better than the above solutions.

If the compiler is either clang or icc under Unix, we:
- run find_program to look for g++
- get the version of g++ (execute_process with --version and regex to extract version)
- check that the version is sufficient
- pass that g++ to clang/icc with --gcc-toolchain/--gcc-name

For clang/icc inside VisualStudio we check its version.

I think this is better because:
- We can report which libstdc++ is being used (location and version)
- The build is more reproducible (without this if the PATH changes between builds than clang/icc use a different libstdc++ based on the different gcc in the PATH). Now the gcc would end up in CMakeCache.txt and thus reproducible.
- It makes it easier for the user to specify the libstd++ (and easier for us to document)

#7 Updated by Roland Schulz 7 months ago

I still think we should use the "find g++" method. But in case we want to determine the libstdc++ version from cmake this does it:

set(WORK_DIR ${CMAKE_BINARY_DIR}${CMAKE_FILES_DIRECTORY}/FindLibStdCpp)
file(WRITE "${WORK_DIR}/glibcxx.cc" "#include <bits/c++config.h>
GLIBCXX = __GLIBCXX__ _GLIBCXX_RELEASE")

string(TOUPPER "${CMAKE_BUILD_TYPE}" _cmake_build_type)
execute_process(COMMAND "${CMAKE_CXX_COMPILER}" "${CMAKE_CXX_FLAGS}" "${CMAKE_CXX_FLAGS_${_cmake_build_type}}" -E -P "${WORK_DIR}/glibcxx.cc" OUTPUT_VARIABLE GLIBCXX)

if(NOT "${GLIBCXX}" MATCHES "GLIBCXX = ([0-9]+) ([A-Z_0-9]+)")
    return() #Compiler isn't using libstdc++
endif()
set(GLIBCXX ${CMAKE_MATCH_1})
set(GLIBCXX_RELEASE ${CMAKE_MATCH_2})

if(NOT ${GLIBCXX_RELEASE} MATCHES "[0-9]+")
    if (GLIBCXX EQUAL 20160427 OR GLIBCXX GREATER_EQUAL 20160822 AND NOT GLIBCXX EQUAL 20171010) # ==6.1 or >=6.2 but not 5.5
       set(GLIBCXX_RELEASE 6)
    elseif (GLIBCXX EQUAL 20150422 OR GLIBCXX GREATER_EQUAL 20150716 AND NOT GLIBCXX EQUAL 20160803) # ==5.1 or >=5.2 but not 4.9.4
       set(GLIBCXX_RELEASE 5)
    else()
       set(GLIBCXX_RELEASE 4) #or smaller
    endif()
endif()

message("Compiler is using libstdc++ ${GLIBCXX_RELEASE}.")

#8 Updated by Gerrit Code Review Bot 7 months ago

Gerrit received a related patchset '1' for Issue #2842.
Uploader: Roland Schulz ()
Change-Id: gromacs~master~I855e16a6d4bd670cfb7acd6ea5c740f3a1b226bf
Gerrit URL: https://gerrit.gromacs.org/9051

#9 Updated by Mark Abraham 7 months ago

  • Parent task set to #2831

#10 Updated by Roland Schulz 6 months ago

  • Status changed from New to Resolved

#11 Updated by Paul Bauer 5 months ago

  • Status changed from Resolved to Closed

Also available in: Atom PDF