This is an archive of the discontinued LLVM Phabricator instance.

Differential D97575

[runtimes] Use standalone build only for compiler-rt
ClosedPublic

Authored by phosek on Feb 26 2021, 12:55 PM.

Details

Reviewers
beanz
smeenai
ldionne
jdoerfert
Commits
rG6e3946c9f558: [runtimes] Use standalone build only for compiler-rt
rG4e421b2323b3: [runtimes] Use standalone build only for compiler-rt
Summary

compiler-rt needs to use standalone build because of the assumptions
made by its build, but other runtimes can use non-standalone build.

Diff Detail

Event Timeline

phosek created this revision.Feb 26 2021, 12:55 PM
Herald added subscribers: mgorny, dberris. · View Herald TranscriptFeb 26 2021, 12:55 PM
phosek requested review of this revision.Feb 26 2021, 12:55 PM
Herald added a reviewer: jdoerfert. · View Herald TranscriptFeb 26 2021, 12:55 PM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added subscribers: Restricted Project, sstefan1. · View Herald Transcript
ldionne added a comment.Feb 26 2021, 12:57 PM

This LGTM except for the comment nitpick, but honestly I'm not entirely sure how to test this. Do you know if the current pre-merge checks run the runtimes build?

runtimes/CMakeLists.txt
113

I think the sentence isn't finished?

phosek updated this revision to Diff 326782.Feb 26 2021, 1:03 PM
phosek marked an inline comment as done.Feb 26 2021, 1:05 PM
In D97575#2591077, @ldionne wrote:

Do you know if the current pre-merge checks run the runtimes build?

AFAIK no, I'm testing this locally for now. The only builder that uses the runtimes build is the Fuchsia (post-submit) one but it's offline right now as we're upgrading the OS. I can hold-off submitting this until they're back online. We also have our downstream builders that use runtimes build which would catch any potential issues.

phosek added a comment.Mar 1 2021, 3:30 PM
In D97575#2591101, @phosek wrote:
In D97575#2591077, @ldionne wrote:

Do you know if the current pre-merge checks run the runtimes build?

AFAIK no, I'm testing this locally for now. The only builder that uses the runtimes build is the Fuchsia (post-submit) one but it's offline right now as we're upgrading the OS. I can hold-off submitting this until they're back online. We also have our downstream builders that use runtimes build which would catch any potential issues.

This seems to be working for me locally so I'd prefer to land this as is (and roll back in case of a downstream issues).

ldionne accepted this revision.Mar 1 2021, 3:37 PM

LGTM

This revision is now accepted and ready to land.Mar 1 2021, 3:37 PM
This revision was landed with ongoing or failed builds.Mar 2 2021, 4:21 PM
Closed by commit rG4e421b2323b3: [runtimes] Use standalone build only for compiler-rt (authored by phosek). · Explain Why
This revision was automatically updated to reflect the committed changes.
phosek added a commit: rG4e421b2323b3: [runtimes] Use standalone build only for compiler-rt.
phosek added a reverting change: rGb3ac90da1df4: Revert "[runtimes] Use standalone build only for compiler-rt".Mar 2 2021, 5:00 PM
phosek updated this revision to Diff 327679.Mar 2 2021, 11:26 PM
phosek reopened this revision.Mar 3 2021, 12:05 AM
phosek updated this revision to Diff 327687.
This revision is now accepted and ready to land.Mar 3 2021, 12:05 AM
This revision was landed with ongoing or failed builds.Mar 3 2021, 12:06 AM
Closed by commit rG6e3946c9f558: [runtimes] Use standalone build only for compiler-rt (authored by phosek). · Explain Why
This revision was automatically updated to reflect the committed changes.
phosek added a commit: rG6e3946c9f558: [runtimes] Use standalone build only for compiler-rt.
amyk added a subscriber: amyk.Mar 4 2021, 6:06 AM

Hi,

I believe this commit is causing failures on the following PowerPC buildbots:
https://lab.llvm.org/buildbot/#/builders/19/builds/2678
https://lab.llvm.org/buildbot/#/builders/18/builds/868

An example of the errors I am seeing:

CMake Error at /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/libcxx/benchmarks/CMakeLists.txt:198 (include):
  include could not find load file:
    AddLLVM
CMake Error at /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/libcxx/benchmarks/CMakeLists.txt:204 (configure_lit_site_cfg):
  Unknown CMake command "configure_lit_site_cfg".
[ 28%] Building IntrinsicsVE.h...
-- Configuring incomplete, errors occurred!
See also "/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-bins/CMakeFiles/CMakeOutput.log".
See also "/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-bins/CMakeFiles/CMakeError.log".
make[3]: *** [projects/compiler-rt/lib/tsan/CMakeFiles/libcxx_tsan_powerpc64-build.dir/build.make:87: projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-stamps/libcxx_tsan_powerpc64-configure] Error 1
make[2]: *** [CMakeFiles/Makefile2:35496: projects/compiler-rt/lib/tsan/CMakeFiles/libcxx_tsan_powerpc64-build.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....
[ 28%] Built target ClangARMMveBuiltinCG
[ 30%] Built target LLVMMC
[ 30%] Built target ClangDriverOptions
[ 30%] Built target ClangARMSveSemaRangeChecks
[ 30%] Built target ClangARMSveBuiltins
[ 30%] Built target ClangARMSveBuiltinCG
[ 30%] Built target intrinsics_gen
[ 36%] Built target clang-resource-headers
make[1]: *** [CMakeFiles/Makefile2:37499: projects/compiler-rt/test/tsan/CMakeFiles/check-tsan.dir/rule] Error 2
make: *** [Makefile:10086: check-tsan] Error 2

Do you by any chance have an idea of a solution for this? Otherwise, would it be possible to pull this patch?

amyk added a comment.Mar 4 2021, 10:59 AM
In D97575#2603354, @amyk wrote:

Hi,

I believe this commit is causing failures on the following PowerPC buildbots:
https://lab.llvm.org/buildbot/#/builders/19/builds/2678
https://lab.llvm.org/buildbot/#/builders/18/builds/868

An example of the errors I am seeing:

CMake Error at /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/libcxx/benchmarks/CMakeLists.txt:198 (include):
  include could not find load file:
    AddLLVM
CMake Error at /home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/llvm-project/libcxx/benchmarks/CMakeLists.txt:204 (configure_lit_site_cfg):
  Unknown CMake command "configure_lit_site_cfg".
[ 28%] Building IntrinsicsVE.h...
-- Configuring incomplete, errors occurred!
See also "/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-bins/CMakeFiles/CMakeOutput.log".
See also "/home/buildbots/ppc64be-sanitizer/sanitizer-ppc64be/build/clang_build/projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-bins/CMakeFiles/CMakeError.log".
make[3]: *** [projects/compiler-rt/lib/tsan/CMakeFiles/libcxx_tsan_powerpc64-build.dir/build.make:87: projects/compiler-rt/lib/tsan/libcxx_tsan_powerpc64-stamps/libcxx_tsan_powerpc64-configure] Error 1
make[2]: *** [CMakeFiles/Makefile2:35496: projects/compiler-rt/lib/tsan/CMakeFiles/libcxx_tsan_powerpc64-build.dir/all] Error 2
make[2]: *** Waiting for unfinished jobs....
[ 28%] Built target ClangARMMveBuiltinCG
[ 30%] Built target LLVMMC
[ 30%] Built target ClangDriverOptions
[ 30%] Built target ClangARMSveSemaRangeChecks
[ 30%] Built target ClangARMSveBuiltins
[ 30%] Built target ClangARMSveBuiltinCG
[ 30%] Built target intrinsics_gen
[ 36%] Built target clang-resource-headers
make[1]: *** [CMakeFiles/Makefile2:37499: projects/compiler-rt/test/tsan/CMakeFiles/check-tsan.dir/rule] Error 2
make: *** [Makefile:10086: check-tsan] Error 2

Do you by any chance have an idea of a solution for this? Otherwise, would it be possible to pull this patch?

I believe I had to force a clean build.
I was able to get a successful build on the sanitizer-ppc64le-linux bot: https://lab.llvm.org/buildbot/#/builders/19/builds/2713

phosek added a comment.Mar 4 2021, 11:33 AM

Great, thank you for looking into it. I tried to reproduce it locally but unsuccessfully, it sounds like it was just a stale build state.

Revision Contents

PathSize
compiler-rt/
cmake/
Modules/
CustomLibcxx/
7 lines
runtimes/
19 lines

Diff 327688

compiler-rt/cmake/Modules/CustomLibcxx/CMakeLists.txt

# TODO(phosek): We should use the runtimes build instead configured with
# LLVM_ENABLE_RUNTIMES=libcxxabi;libcxx to avoid duplication of logic.
cmake_minimum_required(VERSION 3.13.4) cmake_minimum_required(VERSION 3.13.4)
project(custom-libcxx C CXX) project(custom-libcxx C CXX)
find_package(Python3 REQUIRED COMPONENTS Interpreter)
# Build static libcxxabi. # Build static libcxxabi.
set(LIBCXXABI_STANDALONE_BUILD 1)
set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "") set(LIBCXXABI_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXXABI_ENABLE_EXCEPTIONS ON CACHE BOOL "") set(LIBCXXABI_ENABLE_EXCEPTIONS ON CACHE BOOL "")
set(LIBCXXABI_HERMETIC_STATIC_LIBRARY ON CACHE STRING "") set(LIBCXXABI_HERMETIC_STATIC_LIBRARY ON CACHE STRING "")
set(LIBCXXABI_LIBCXX_PATH ${COMPILER_RT_LIBCXX_PATH} CACHE PATH "") set(LIBCXXABI_LIBCXX_PATH ${COMPILER_RT_LIBCXX_PATH} CACHE PATH "")
set(LIBCXXABI_INCLUDE_TESTS OFF CACHE BOOL "") set(LIBCXXABI_INCLUDE_TESTS OFF CACHE BOOL "")
add_subdirectory(${COMPILER_RT_LIBCXXABI_PATH} ${CMAKE_CURRENT_BINARY_DIR}/cxxabi) add_subdirectory(${COMPILER_RT_LIBCXXABI_PATH} ${CMAKE_CURRENT_BINARY_DIR}/cxxabi)
# Build static libcxx without exceptions. # Build static libcxx without exceptions.
set(LIBCXX_STANDALONE_BUILD 1)
set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "") set(LIBCXX_ENABLE_EXPERIMENTAL_LIBRARY OFF CACHE BOOL "")
set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "") set(LIBCXX_ENABLE_SHARED OFF CACHE BOOL "")
set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "") set(LIBCXX_ENABLE_EXCEPTIONS OFF CACHE BOOL "")
set(LIBCXX_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "") set(LIBCXX_HERMETIC_STATIC_LIBRARY ON CACHE BOOL "")
# Use above libcxxabi. # Use above libcxxabi.
set(LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "") set(LIBCXX_CXX_ABI "libcxxabi" CACHE STRING "")
set(LIBCXX_CXX_ABI_INTREE 1) set(LIBCXX_CXX_ABI_INTREE 1)
set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "") set(LIBCXX_ENABLE_STATIC_ABI_LIBRARY ON CACHE BOOL "")
set(LIBCXX_CXX_ABI_INCLUDE_PATHS ${COMPILER_RT_LIBCXXABI_PATH}/include CACHE PATH "") set(LIBCXX_CXX_ABI_INCLUDE_PATHS ${COMPILER_RT_LIBCXXABI_PATH}/include CACHE PATH "")
add_subdirectory(${COMPILER_RT_LIBCXX_PATH} ${CMAKE_CURRENT_BINARY_DIR}/cxx) add_subdirectory(${COMPILER_RT_LIBCXX_PATH} ${CMAKE_CURRENT_BINARY_DIR}/cxx)

runtimes/CMakeLists.txt

Show First 20 Lines • Show All 76 Lines • ▼ Show 20 Lines
include(CheckLibraryExists) include(CheckLibraryExists)
include(CheckCCompilerFlag) include(CheckCCompilerFlag)
# Disable use of the installed C++ standard library when building runtimes. If # Disable use of the installed C++ standard library when building runtimes. If
# MSVC is true, we must be using the clang-cl driver, which doesn't understand # MSVC is true, we must be using the clang-cl driver, which doesn't understand
# these flags. # these flags.
if (NOT MSVC) if (NOT MSVC)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdinc++ -nostdlib++") set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -nostdinc++ -nostdlib++")
if (LLVM_EXTERNAL_LIBCXX_SOURCE_DIR AND "libcxx" IN_LIST LLVM_ENABLE_RUNTIMES)
set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -isystem ${LLVM_EXTERNAL_LIBCXX_SOURCE_DIR}/include")
endif()
endif() endif()
# Avoid checking whether the compiler is working. # Avoid checking whether the compiler is working.
set(LLVM_COMPILER_CHECKED ON) set(LLVM_COMPILER_CHECKED ON)
# Handle common options used by all runtimes. # Handle common options used by all runtimes.
include(AddLLVM) include(AddLLVM)
include(HandleLLVMOptions) include(HandleLLVMOptions)
include(FindPythonInterp)
# Remove the -nostdlib++ option we've added earlier. find_package(Python3 REQUIRED COMPONENTS Interpreter)
string(REPLACE "-nostdlib++" "" CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS}")
# Use libtool instead of ar if you are both on an Apple host, and targeting Apple. # Use libtool instead of ar if you are both on an Apple host, and targeting Apple.
if(CMAKE_HOST_APPLE AND APPLE) if(CMAKE_HOST_APPLE AND APPLE)
include(UseLibtool) include(UseLibtool)
endif() endif()
# This can be used to detect whether we're in the runtimes build. # This can be used to detect whether we're in the runtimes build.
set(RUNTIMES_BUILD ON) set(RUNTIMES_BUILD ON)
foreach(entry ${runtimes}) foreach(entry ${runtimes})
get_filename_component(projName ${entry} NAME) get_filename_component(projName ${entry} NAME)
# TODO: Clean this up as part of an interface standardization # TODO: Clean this up as part of an interface standardization
string(REPLACE "-" "_" canon_name ${projName}) string(REPLACE "-" "_" canon_name ${projName})
string(TOUPPER ${canon_name} canon_name) string(TOUPPER ${canon_name} canon_name)
# The subdirectories need to treat this as standalone builds. D57992 tried # TODO: compiler-rt has to use standalone build for now. We tried to remove
# to get rid of this, but the runtimes treat *_STANDALONE_BUILD=OFF as if # this in D57992 but this broke the build because compiler-rt assumes that
# llvm & clang are configured in the same CMake, and setup dependencies # LLVM and Clang are configured in the same build to set up dependencies. We
ldionneUnsubmitted
Done
ReplyInline Actions

I think the sentence isn't finished?

ldionne: I think the sentence isn't finished?
# against their targets. OpenMP has fixed the issue so we don't set the # should clean up the compiler-rt build and remove this eventually.
# variable. if ("${canon_name}" STREQUAL "COMPILER_RT")
if (NOT ${canon_name} STREQUAL "OPENMP")
set(${canon_name}_STANDALONE_BUILD ON) set(${canon_name}_STANDALONE_BUILD ON)
endif() endif()
if(LLVM_RUNTIMES_LIBDIR_SUBDIR) if(LLVM_RUNTIMES_LIBDIR_SUBDIR)
set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE) set(${canon_name}_LIBDIR_SUBDIR "${LLVM_RUNTIMES_LIBDIR_SUBDIR}" CACHE STRING "" FORCE)
endif() endif()
# Setting a variable to let sub-projects detect which other projects # Setting a variable to let sub-projects detect which other projects
▲ Show 20 Lines • Show All 73 Lines • Show Last 20 Lines