-
Notifications
You must be signed in to change notification settings - Fork 15.9k
[LLVM][CMake][NFC] Use generator expression to separate CXXFLAGS #173869
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This avoids looking at the individual sources for mixed C/C++ libraries. The previous code was written ~2014. Generator expressions were added in CMake 3.3 (2015). We currently require CMake 3.20 and therefore can rely on more modern features.
nikic
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/20837 Here is the relevant piece of the build log for the reference |
|
We have a bot to build the rust compiler with LLVM close to HEAD. Looks like starting from this commit, we're seeing some new linker errors like: Some sort of RTTI mismatch between how we build LLVM and the rust llvm-wrapper. The builder uses this configuration: Adding |
|
@krasimirgg Rust shouldn't need LLVM built with RTTI. I'm not sure how this PR changed things. It looks like https://github.com/rust-lang/rust/blob/main/compiler/rustc_llvm/build.rs (the LLVM bindings build) doesn't set It would be good to get a full build log with the compiler invocations, so the used flags are visible. |
|
llvm-config uses And this amount of trouble just so that two unittests (/flang/unittests/Evaluate, /clang/unittests/Interpreter/ExceptionTests) can compile with exceptions (which, btw, also seem to work fine without RTTI) -- otherwise we could just set the flags globally and wouldn't need this llvm_update_compile_flags hack. |
|
A tangentially related questions is whether (And when using |
Fix for #173869. If there's no strong reason, we should get rid of per-target RTTI later.
Fix for llvm/llvm-project#173869. If there's no strong reason, we should get rid of per-target RTTI later.
Fix for llvm#173869. If there's no strong reason, we should get rid of per-target RTTI later.
…m#173869) This avoids looking at the individual sources for mixed C/C++ libraries. The previous code was written ~2014. Generator expressions were added in CMake 3.3 (2015). We currently require CMake 3.20 and therefore can rely on more modern features. Apart from simplifying the code, this is preliminary work to make more use of pre-compiled headers (llvm#173868).
Fix for llvm#173869. If there's no strong reason, we should get rid of per-target RTTI later.
Fix for llvm/llvm-project#173869. If there's no strong reason, we should get rid of per-target RTTI later.
Fix for llvm/llvm-project#173869. If there's no strong reason, we should get rid of per-target RTTI later. (cherry picked from commit d1b88ca)
#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then #174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM.
…(#176195) llvm/llvm-project#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm/llvm-project#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM.
llvm#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM.
llvm#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM.
llvm#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM. (cherry picked from commit 9bbea75)
…(#176195) llvm/llvm-project#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm/llvm-project#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM. (cherry picked from commit 9bbea75)
llvm#173869 accidentally dropped rtti and eh flags from `llvm-config --cxxflags`. Then llvm#174084 restored the rtti flags. The eh flags were not included with the rationale that they are not ABI relevant. This PR restores the eh flags as well. While they are not strictly necessary, I believe that code that directly interfaces with LLVM almost certainly does not want to build with exceptions if LLVM is not built with exceptions. Building in the peculiar `-fexceptions -fno-rtti` configuration is rarely useful and likely not intended. On MacOS, this is also relevant because it's not possible to use C++17 headers without `-fno-exceptions` when using older deployment targets. In that configuration, `-fno-exceptions` is required to interact with LLVM.
This avoids looking at the individual sources for mixed C/C++ libraries.
The previous code was written ~2014. Generator expressions were added in CMake 3.3 (2015). We currently require CMake 3.20 and therefore can rely on more modern features.
Apart from simplifying the code, this is preliminary work to make more use of pre-compiled headers (#173868).