Important: New toolchain requirements to build LLVM will (most likely) be landing within a week. Prepare your buildbots

Project Infrastructure
,
1

Hi,

After discussions in https://discourse.llvm.org/t/rfc-increasing-the-gcc-and-clang-requirements-to-support-c-17-in-llvm

And also review comments in: :gear: D122976 Bump minimum toolchain version (llvm.org)

Update: Commit has landed: Bump minimum toolchain version · llvm/llvm-project@4c72deb (github.com)

It seems likely that we will raise the toolchain version requirements later this week or beginning of next one. This message is a heads up for the buildbot owners and other people that build LLVM directly from main to make preparations for this migration.

The new requirements are as follows:

  • Clang 5.0
  • Apple Clang 9.3
  • GCC 7.1
  • Visual Studio 2019 16.7 (This is already the requirement in main)

The current time-plan looks like this:

  • Let the diff be reviewed and discussed during this week 4/4 - 8/4
  • The soft error will land in main after this.
  • This soft error will then live in main until LLVM 15 has been branched
  • Hard error will be landed in main after the branch
  • CXX_VERSION will be changed to C+17 and developers are allowed to use C++17 in main.
  • LLVM 16.x will be the first version hard requiring these changes.

Please keep comments about this change to the phabricator review or the first forum post. This one is mainly used as an announcement thread where I will update the status as we go along.

Thanks,
Tobias

8 Likes
2

AFAICT the Visual Studio minimum has not changed?

1 Like
3

Yeah that’s correct, it was already raised earlier. I will edit the post and note that it hasn’t changed.

6
7

Update: The commit for soft errors when using an older toolchain has now landed.

Nothing more will happen here until LLVM 15 has been branched, then the soft error will transform into a hard error on main and CXX_STANDARD will be bumped to 17.

Thanks everyone providing input and helping me push this forward.

8

It’s been many hours since the LLVM 15 branch was created. I guess this is a good moment to bring this back to the fore…

9

Yep. I plan to open a diff with this change soon. I haven’t forgotten it :slight_smile:

1 Like
10

FYI - we ran into an issue trying to sequence a CXX_STANDARD upgrade to 17 (one of our deps started landing code that required it) and hit a sharp edge in both CMake and GCC. Workaround here: https://github.com/iree-org/iree/pull/9924 (I expect anyone who is consuming LLVM and becomes mis-aligned on the standard version may hit this as soon as they integrate the patch bumping it). Iiuc, there are some CMake policies in play that affect how normal variables shadow later defined cache variables and in some combination can cause LLVM’s cache variable to take precedence over a setting at a higher level.

The specific GCC bug that gets triggered on a mismatch is: 101957 – Multiple definition of static constexpr data member when C++11 and C++17 objects are mixed

In our case, it was causing a multiple definition error on a static constexpr int at class scope within MLIR. tl;dr - if you start getting multiple definition errors on GCC after this switch and are using LLVM as a library, double check that the CXX_STANDARD settings are being applied identically to both the code including LLVM headers and LLVM itself (which due to CMake issues can become mis-aligned easily).

11

I have pushed a diff here now to update the CXX_STANDARD to C++17 and also to make the soft toolchain requirements hard: :gear: D130689 [LLVM] Update C++ standard to 17

Please have a look!

12

Hi!

It looks like the toolchain shipped as Apple Clang 9.3 doesn’t have full support for C++17 features, particularly std::variant and std::optional (see Compiler support for C++17 - cppreference.com, it claims support for those comes with 10.0.0).

ATM, LLVM already accumulated usage of std::variant in its various components (TableGen, DenseMap, CodeGen, flang, pseudo, MLIR) as none of the buildbots seem to be testing this configuration.

I was reading RFC: Increasing the GCC and Clang requirements to support C++17 in LLVM - #10 by reinterpretcast, couldn’t see any specific justifications neither there or here about choosing apple clang 9.0.3. Also only guidance I can see for which c++17 features to use is at C++ 17 in LLVM code base! - #4 by tobiashieta, which doesn’t really mention what those are. Apparently variant and optional needs to be excluded, at the very least, which seems very unfortunate.

Any chance we can revisit this decision and bump minimum apple clang toolchain requirement to 10.0.0, be more specific about the standard library requirements in Getting Started with the LLVM System — LLVM 16.0.0git documentation or have a specific list of allowed c++17 features (and buildbots/checks enforcing them)?

This was brought up by @tschuett in ⚙ D135953 [IncludeCleaner] Introduce decl to location mapping.

13

It would be great to bump the minimum to Apple Clang 10.0.0. We could get rid of all the unions and eventually migrate from the LLVM optional to the std one.

14

I think the extent of my investigation here was to check which Apple Clang version matched the Clang version we were bumping to. But I am no expert on Apple Clang and no one objected back then when the exact versions were discussed.

I think it makes sense to bump it to 10.0.0, but I think you should create a separate RFC for that and make sure that no one that actually knows the apple clang versions and usage disagrees. I think it’s better if we try to do this before we branch to LLVM 16 so that we don’t bump toolchain requirements in two subsequent versions of LLVM.

15

@tschuett would you be OK with proposing that RFC? I am more than happy to send a draft, but I don’t really have the means to test or understand the issues that might come up without spending a significant amount of time.

16

The issue is that there is already std::variant in the codebase. However, it is not supported by the minimum toolchain requirements.

17

right, and the solution proposed is giving community an RFC for bumping minimum requirements to apple clang 10.0.0, instead of 9.0.3. I was asking if you could follow up on that