llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-semantics Author: Alexis Engelke (aengelke) <details> <summary>Changes</summary> If we compile with RTTI disabled, don't override it again for any target. This reduces the number of different build flags. Mixing RTTI and non-RTTI objects is problematic anyway on some platforms, e.g. MSVC. - LLVM_REQUIRES_* are per-target flags that are never set globally. Yet, some files used these (undefined) flags for some logic. This patch removes these dead checks/unconditionally executes the logic. Note that the references *.exports files are empty, so there is no need to make related logic conditional on MSVC. - gtest automatically determines GTEST_HAS_RTTI from pre-defined compiler macros, there is no need to explicitly define this and especially no need to define this for every single source file. - add_flang_nongtest_unittest never disables RTTI/exceptions and just uses whatever the compiler defaults to (typically both enabled). There should be no difference in removing the forced RTTI enabling, especially given that the test doesn't use exceptions/RTTI at all. - The *only* (non-runtime) user of exceptions in-tree is clang/unittests/Interpreter/ExceptionTests. This test only uses catch(...), which should work fine without RTTI on all platforms. After this, we could move the RTTI enable/disable flag to the general CMAKE_CXX_FLAGS. This is preliminary work for using pre-compiled headers for unit tests, where llvm_gtest and the gtest PCH should be compiled without RTTI to permit PCH reuse for most (all-but-one) executables. --- Full diff: https://github.com/llvm/llvm-project/pull/174201.diff 8 Files Affected: - (modified) clang/examples/LLVMPrintFunctionNames/CMakeLists.txt (+2-10) - (modified) clang/examples/PrintFunctionNames/CMakeLists.txt (+2-10) - (modified) clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt (+4-1) - (modified) flang/unittests/Evaluate/CMakeLists.txt (+2-1) - (modified) llvm/cmake/modules/AddLLVM.cmake (+1-14) - (modified) llvm/tools/bugpoint-passes/CMakeLists.txt (+2-7) - (modified) offload/unittests/Conformance/lib/CMakeLists.txt (-4) - (modified) third-party/unittest/CMakeLists.txt (-3) ``````````diff diff --git a/clang/examples/LLVMPrintFunctionNames/CMakeLists.txt b/clang/examples/LLVMPrintFunctionNames/CMakeLists.txt index 61e7a7842e2f3..fbab0f8773bc8 100644 --- a/clang/examples/LLVMPrintFunctionNames/CMakeLists.txt +++ b/clang/examples/LLVMPrintFunctionNames/CMakeLists.txt @@ -1,13 +1,5 @@ -# If we don't need RTTI or EH, there's no reason to export anything -# from the plugin. -if(NOT MSVC) # MSVC mangles symbols differently, and - # PrintLLVMFunctionNames.export contains C++ symbols. - if(NOT LLVM_REQUIRES_RTTI) - if(NOT LLVM_REQUIRES_EH) - set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/LLVMPrintFunctionNames.exports) - endif() - endif() -endif() +# There's no reason to export anything from the plugin. +set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/LLVMPrintFunctionNames.exports) add_llvm_library(LLVMPrintFunctionNames MODULE LLVMPrintFunctionNames.cpp PLUGIN_TOOL clang) diff --git a/clang/examples/PrintFunctionNames/CMakeLists.txt b/clang/examples/PrintFunctionNames/CMakeLists.txt index 28da363729f3a..3668a3cdccf8e 100644 --- a/clang/examples/PrintFunctionNames/CMakeLists.txt +++ b/clang/examples/PrintFunctionNames/CMakeLists.txt @@ -1,13 +1,5 @@ -# If we don't need RTTI or EH, there's no reason to export anything -# from the plugin. -if( NOT MSVC ) # MSVC mangles symbols differently, and - # PrintFunctionNames.export contains C++ symbols. - if( NOT LLVM_REQUIRES_RTTI ) - if( NOT LLVM_REQUIRES_EH ) - set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/PrintFunctionNames.exports) - endif() - endif() -endif() +# There's no reason to export anything from the plugin. +set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/PrintFunctionNames.exports) add_llvm_library(PrintFunctionNames MODULE PrintFunctionNames.cpp PLUGIN_TOOL clang) diff --git a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt index dfd94d8e6442c..fd91c56caa305 100644 --- a/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt +++ b/clang/unittests/Interpreter/ExceptionTests/CMakeLists.txt @@ -1,7 +1,10 @@ # The interpreter can throw an exception from user input. The test binary needs # to be compiled with exception support to catch the thrown exception. +# +# NOTE: we compile with exceptions enabled, but (possibly) with RTTI disabled. +# This only works in these limited circumstances where all exception-related +# code is in the same library/executable. set(LLVM_REQUIRES_EH ON) -set(LLVM_REQUIRES_RTTI ON) add_distinct_clang_unittest(ClangReplInterpreterExceptionTests InterpreterExceptionTest.cpp diff --git a/flang/unittests/Evaluate/CMakeLists.txt b/flang/unittests/Evaluate/CMakeLists.txt index ed012828a7258..500be98977f1c 100644 --- a/flang/unittests/Evaluate/CMakeLists.txt +++ b/flang/unittests/Evaluate/CMakeLists.txt @@ -45,8 +45,9 @@ add_flang_nongtest_unittest(logical # IEEE exception flags (different use of the word "exception") # in the actual hardware floating-point status register, so ensure that # C++ exceptions are enabled for this test. +# TODO: add_flang_nongtest_unittest never disables RTTI/exceptions, this should +# not be required at all. set(LLVM_REQUIRES_EH ON) -set(LLVM_REQUIRES_RTTI ON) add_flang_nongtest_unittest(real NonGTestTesting FortranEvaluate diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake index d6acb4f984be9..c81e5cbc323e3 100644 --- a/llvm/cmake/modules/AddLLVM.cmake +++ b/llvm/cmake/modules/AddLLVM.cmake @@ -25,10 +25,6 @@ function(llvm_update_compile_flags name) # LLVM_REQUIRES_EH is an internal flag that individual targets can use to # force EH if(LLVM_REQUIRES_EH OR LLVM_ENABLE_EH) - if(NOT (LLVM_REQUIRES_RTTI OR LLVM_ENABLE_RTTI)) - message(AUTHOR_WARNING "Exception handling requires RTTI. Enabling RTTI for ${name}") - set(LLVM_REQUIRES_RTTI ON) - endif() if(MSVC) list(APPEND LLVM_COMPILE_CXXFLAGS "/EHsc") endif() @@ -49,12 +45,7 @@ function(llvm_update_compile_flags name) endif() endif() - # LLVM_REQUIRES_RTTI is an internal flag that individual - # targets can use to force RTTI - if(NOT (LLVM_REQUIRES_RTTI OR LLVM_ENABLE_RTTI)) - # TODO: GTEST_HAS_RTTI should be determined automatically, evaluate whether - # the explicit definition is actually required. - list(APPEND LLVM_COMPILE_DEFINITIONS GTEST_HAS_RTTI=0) + if(NOT LLVM_ENABLE_RTTI) list(APPEND LLVM_COMPILE_CXXFLAGS ${LLVM_CXXFLAGS_RTTI_DISABLE}) else() list(APPEND LLVM_COMPILE_CXXFLAGS ${LLVM_CXXFLAGS_RTTI_ENABLE}) @@ -1760,10 +1751,6 @@ function(add_unittest test_suite test_name) list(APPEND LLVM_COMPILE_FLAGS "-Wno-gnu-zero-variadic-macro-arguments") endif() - if (NOT DEFINED LLVM_REQUIRES_RTTI) - set(LLVM_REQUIRES_RTTI OFF) - endif() - list(APPEND LLVM_LINK_COMPONENTS Support) # gtest needs it for raw_ostream add_llvm_executable(${test_name} IGNORE_EXTERNALIZE_DEBUGINFO NO_INSTALL_RPATH ${ARGN}) get_subproject_title(subproject_title) diff --git a/llvm/tools/bugpoint-passes/CMakeLists.txt b/llvm/tools/bugpoint-passes/CMakeLists.txt index 60fc1bde51371..53587a51c5081 100644 --- a/llvm/tools/bugpoint-passes/CMakeLists.txt +++ b/llvm/tools/bugpoint-passes/CMakeLists.txt @@ -2,13 +2,8 @@ if( NOT LLVM_BUILD_TOOLS ) set(EXCLUDE_FROM_ALL ON) endif() -# If we don't need RTTI or EH, there's no reason to export anything -# from this plugin. -if( NOT LLVM_REQUIRES_RTTI ) - if( NOT LLVM_REQUIRES_EH ) - set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/bugpoint.exports) - endif() -endif() +# There's no reason to export anything from the plugin. +set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/bugpoint.exports) if(WIN32 OR CYGWIN OR ZOS) set(LLVM_LINK_COMPONENTS Core Support) diff --git a/offload/unittests/Conformance/lib/CMakeLists.txt b/offload/unittests/Conformance/lib/CMakeLists.txt index 8e86f101729ad..0d042ead4b5ac 100644 --- a/offload/unittests/Conformance/lib/CMakeLists.txt +++ b/offload/unittests/Conformance/lib/CMakeLists.txt @@ -3,9 +3,5 @@ add_library(MathTest STATIC target_include_directories(MathTest PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../include") -if(NOT LLVM_REQUIRES_RTTI) - target_compile_options(MathTest PUBLIC -fno-rtti) -endif() - include(FindLibcCommonUtils) target_link_libraries(MathTest PUBLIC LLVMOffload LLVMSupport LLVMDemangle llvm-libc-common-utilities) diff --git a/third-party/unittest/CMakeLists.txt b/third-party/unittest/CMakeLists.txt index 79535a1de4616..4dc975321ef41 100644 --- a/third-party/unittest/CMakeLists.txt +++ b/third-party/unittest/CMakeLists.txt @@ -59,9 +59,6 @@ if(CXX_SUPPORTS_COVERED_SWITCH_DEFAULT_FLAG) add_definitions("-Wno-covered-switch-default") endif() -set(LLVM_REQUIRES_RTTI 1) -add_definitions( -DGTEST_HAS_RTTI=0 ) - if (HAVE_LIBPTHREAD) list(APPEND LIBS pthread) endif() `````````` </details> https://github.com/llvm/llvm-project/pull/174201 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
