llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.39k stars 12.15k forks source link

[libc] consider using runtimes/CMakeLists.txt as base (rather than llvm/CMakeLists.txt) #78479

Open nickdesaulniers opened 10 months ago

nickdesaulniers commented 10 months ago

we've observed issues in the past where flags set in llvm's cmake files end up polluting toolchain command line options for libc.

Via discussions with @ldionne on discord and https://github.com/llvm/llvm-project/pull/77810#issuecomment-1890061319

Yes, I've run into such pollution many times This was one of the many motivating reasons for switching from LLVM_ENABLE_PROJECTS to LLVM_ENABLE_RUNTIMES, and using runtimes/CMakeLists.txt instead of llvm/CMakeLists.txt as a CMake root It didn't solve everything, but at least it insulated the runtimes from arbitrary flags being added in other LLVM projects We still have this issue due to e.g. HandleLLVMOptions.cmake I think eventually the solution would be to ban global flags entirely. Modern CMake doesn't need that, it can define targets with flags and have them propagate to dependencies. It's a lot cleaner and safer For most projects under the LLVM umbrella, being polluted by flags is an annoyance. For the runtimes, this can be actually dangerous cause you need to control exactly what flags you use for building.

I think this makes sense, however I'll note that other things can be polluted by the "rest" of LLVM as well: Link flags CMake variables like CMAKE_POSITION_INDEPENDENT_CODE Other compiler flags that are not -D options This is better than nothing, but I think the full fix is to gradually refactor the CMake files used by runtimes to avoid setting anything global.

We would have to update documentation, too. cc @petrhosek

llvmbot commented 10 months ago

@llvm/issue-subscribers-libc

Author: Nick Desaulniers (nickdesaulniers)

we've observed issues in the past where flags set in llvm's cmake files end up polluting toolchain command line options for libc. Via discussions with @ldionne on discord and https://github.com/llvm/llvm-project/pull/77810#issuecomment-1890061319 > Yes, I've run into such pollution many times > This was one of the many motivating reasons for switching from LLVM_ENABLE_PROJECTS to LLVM_ENABLE_RUNTIMES, and using runtimes/CMakeLists.txt instead of llvm/CMakeLists.txt as a CMake root > It didn't solve everything, but at least it insulated the runtimes from arbitrary flags being added in other LLVM projects > We still have this issue due to e.g. HandleLLVMOptions.cmake > I think eventually the solution would be to ban global flags entirely. Modern CMake doesn't need that, it can define targets with flags and have them propagate to dependencies. It's a lot cleaner and safer > For most projects under the LLVM umbrella, being polluted by flags is an annoyance. For the runtimes, this can be actually dangerous cause you need to control exactly what flags you use for building. > I think this makes sense, however I'll note that other things can be polluted by the "rest" of LLVM as well: > Link flags > CMake variables like CMAKE_POSITION_INDEPENDENT_CODE > Other compiler flags that are not -D options > This is better than nothing, but I think the full fix is to gradually refactor the CMake files used by runtimes to avoid setting anything global. We would have to update documentation, too. cc @petrhosek
nickdesaulniers commented 1 week ago

Ok, so looking at the buildbots, the are a mix of build bots that are using -DLLVM_ENABLE_PROJECTS=libc rather than -DLLVM_ENABLE_RUNTIMES=libc. This is based on the name of the buildbot containing "runtimes" or not.

https://github.com/llvm/llvm-zorg/blob/main/zorg/buildbot/builders/annotated/libc-linux.py

Actually, all of the buildbots except libc-x86_64-debian-dbg-runtimes-build are using enable projects.

So I think doing this in a few steps would look like:

nickdesaulniers commented 1 week ago

Since libc-x86_64-debian-dbg-runtimes-build is the only enable runtimes bot, I propose that we delete it, then just move all buildbots to use enable runtimes rather than enable projects.

The fullbuilds enable clang and compiler-rt; I'm not sure we need to do that? https://github.com/llvm/llvm-zorg/blob/7ddf53595cbd6bebc63b8e8ee7f0ac3617a25e26/zorg/buildbot/builders/annotated/libc-linux.py#L90C1-L91C1