This is an archive of the discontinued LLVM Phabricator instance.

Use LTO capable linker
ClosedPublic

Authored by winksaville on May 22 2019, 6:29 PM.

Details

Summary

In DistributionExample.cmake be sure we use a LTO
capable linker, the easiest to choose is lld.

Diff Detail

Repository
rL LLVM

Event Timeline

winksaville created this revision.May 22 2019, 6:29 PM
winksaville abandoned this revision.EditedMay 22 2019, 6:47 PM

Chris, I was going to report that libcxxabi_shared was missing as I thought you had added it to in LLVM_ENABLE_RUNTIMES, but you hadn't so abandoning and trying again.

My mistake.

Adding libcxxabi worked and ninja stage2-distribution succeeded but I then ran ninja check-all and from within stage2-bins/ but that failed:

[1072/1526] cd /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi/tools/clang/stage2-bins/runtimes/runtimes-bins && /usr/bin/cmake --build /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi/tools/clang/stage2-bins/runtimes/runtimes-bins/ --target check-runtimes --config RelWithDebInfo
ninja: error: '/home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi/tools/clang/stage2-bins/lib/libgtest.a', needed by 'compiler-rt/lib/asan/tests/ASAN_INST_TEST_OBJECTS.gtest-all.cc.x86_64-calls.o', missing and no known rule to make it
FAILED: runtimes/CMakeFiles/check-runtimes 
cd /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi/tools/clang/stage2-bins/runtimes/runtimes-bins && /usr/bin/cmake --build /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi/tools/clang/stage2-bins/runtimes/runtimes-bins/ --target check-runtimes --config RelWithDebInfo

I've add gtest_main and gtest to CLANG_BOOTSTRAP_TARGETS as a guess, because that's where check-all is defined:

 # Expose stage2 targets through the stage1 build configuration.
 set(CLANG_BOOTSTRAP_TARGETS
+  gtest_main
+  gtest
   check-all
   check-llvm
   check-clang
   llvm-config
   test-suite
   test-depends
   llvm-test-depends
   clang-test-depends
   distribution
   install-distribution

My guess is likely wrong, what do you advise?

I've add gtest_main and gtest to CLANG_BOOTSTRAP_TARGETS as a guess, because that's where check-all is defined:
...
My guess is likely wrong, what do you advise?

Are you still having that issue after rL361436? That should have resolved that problem. The issue isn't that gtest is missing from the bootstrap, but rather that it was missing from the dependencies for the runtime libraries.

Are you still having that issue after rL361436? That should have resolved that problem. The issue isn't that gtest is missing from the bootstrap, but rather that it was missing from the dependencies for the runtime libraries.

Yes, that patch was/is in my master when it failed above:

$ git log -1 -p ed0036796164b3a2a93be8e2707984f57ba94e24
commit ed0036796164b3a2a93be8e2707984f57ba94e24
Author: Chris Bieneman <chris.bieneman@me.com>
Date:   Wed May 22 21:42:06 2019 +0000

    [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest
    
    Summary: If we are building the tests for the runtimes we should make them depend on gtest so that gtest is built and ready before we run any of the check-* targets.
    
    Reviewers: phosek, compnerd
    
    Reviewed By: compnerd
    
    Subscribers: mgorny, winksaville, llvm-commits
    
    Tags: #llvm
    
    Differential Revision: https://reviews.llvm.org/D62269
    
    llvm-svn: 361436

diff --git a/llvm/runtimes/CMakeLists.txt b/llvm/runtimes/CMakeLists.txt
index 7bbf0cf26a5..285e1fcae1d 100644
--- a/llvm/runtimes/CMakeLists.txt
+++ b/llvm/runtimes/CMakeLists.txt
@@ -553,6 +553,8 @@ else() # if this is included from LLVM's CMake
           obj2yaml
           sancov
           sanstats
+          gtest_main
+          gtest
         )
       foreach(target ${test_targets} ${SUB_CHECK_TARGETS})
         add_dependencies(${target} ${RUNTIMES_TEST_DEPENDS})

My master is at 74eb76f6:

wink@wink-desktop:~/prgs/llvm/llvm-project (Use-LTO-capable-linker-and-add-libcxxabi)
$ git log -1 master
commit 74eb76f6c31e13551269f711cfccceca92b45783 (upstream/master, origin/master, origin/HEAD, master)
Author: Alex Langford <apl@fb.com>
Date:   Wed May 22 23:01:18 2019 +0000

    [Target] Protect Processes' language runtimes map with a mutex
    
    Summary:
    From what I understand, it's possible for multiple threads to request
    a specific language runtime (e.g. CPPLanguageRuntime). This leads to a data
    race.
    
    Reviewers: jingham, JDevlieghere, compnerd, clayborg
    
    Differential Revision: https://reviews.llvm.org/D62169
    
    llvm-svn: 361442

On top of master I have two commits to add lld, libcxxabi, gtest_main and gtest:

$ git log -2 -p
commit b6b78d5b1d55b7643634ded9412c42dd52782e37 (HEAD -> Use-LTO-capable-linker-and-add-libcxxabi)
Author: Wink Saville <wink@saville.com>
Date:   Thu May 23 08:14:44 2019 -0700

    Add gtest_main and gtest
    
    See if this fixes check all

diff --git a/clang/cmake/caches/DistributionExample.cmake b/clang/cmake/caches/DistributionExample.cmake
index 50fcc09cf07..fd525b8f4ce 100644
--- a/clang/cmake/caches/DistributionExample.cmake
+++ b/clang/cmake/caches/DistributionExample.cmake
@@ -24,6 +24,8 @@ endif()
 
 # Expose stage2 targets through the stage1 build configuration.
 set(CLANG_BOOTSTRAP_TARGETS
+  gtest_main
+  gtest
   check-all
   check-llvm
   check-clang

commit b99e88f595fd3e27e68c18d95f88552cb25aea43 (origin/Use-LTO-capable-linker-and-add-libcxxabi, origin/Use-LTO-capable-linker)
Author: Wink Saville <wink@saville.com>
Date:   Wed May 22 16:23:47 2019 -0700

    Use LTO capable linker and add libcxxabi
    
    In DistributionExample.cmake be sure we use a LTO
    capable linker, the easiest to choose is lld.
    
    Reviewers: beanz
    
    Subscribers: mgorny, mehdi_amini, inglorion, dexonsmith, cfe-commits
    
    Tags: #clang
    
    Differential Revision: https://reviews.llvm.org/D62279

diff --git a/clang/cmake/caches/DistributionExample-stage2.cmake b/clang/cmake/caches/DistributionExample-stage2.cmake
index f4d5d92d1d1..99d5dc0fd2f 100644
--- a/clang/cmake/caches/DistributionExample-stage2.cmake
+++ b/clang/cmake/caches/DistributionExample-stage2.cmake
@@ -2,7 +2,7 @@
 # bootstrap build.
 
 set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld" CACHE STRING "")
-set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" CACHE STRING "")
 
 set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
 
diff --git a/clang/cmake/caches/DistributionExample.cmake b/clang/cmake/caches/DistributionExample.cmake
index 35493edd17f..50fcc09cf07 100644
--- a/clang/cmake/caches/DistributionExample.cmake
+++ b/clang/cmake/caches/DistributionExample.cmake
@@ -2,7 +2,7 @@
 
 #Enable LLVM projects and runtimes
 set(LLVM_ENABLE_PROJECTS "clang;clang-tools-extra;lld" CACHE STRING "")
-set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx" CACHE STRING "")
+set(LLVM_ENABLE_RUNTIMES "compiler-rt;libcxx;libcxxabi" CACHE STRING "")
 
 # Only build the native target in stage1 since it is a throwaway build.
 set(LLVM_TARGETS_TO_BUILD Native CACHE STRING "")
@@ -17,6 +17,11 @@ set(PACKAGE_VENDOR LLVM.org CACHE STRING "")
 # the proper LTO library dependencies can be connected.
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
 
+if (NOT APPLE)
+  # Since LLVM_ENABLE_LTO is ON we need a LTO capable linker
+  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
+endif()
+
 # Expose stage2 targets through the stage1 build configuration.
 set(CLANG_BOOTSTRAP_TARGETS
   check-all

So the commits from your commit to HEAD is:

$ git log --oneline ed0036796164b3a2a93be8e2707984f57ba94e24^..HEAD
b6b78d5b1d5 (HEAD -> Use-LTO-capable-linker-and-add-libcxxabi) Add gtest_main and gtest
b99e88f595f (origin/Use-LTO-capable-linker-and-add-libcxxabi, origin/Use-LTO-capable-linker) Use LTO capable linker and add libcxxabi
74eb76f6c31 (upstream/master, origin/master, origin/HEAD, master) [Target] Protect Processes' language runtimes map with a mutex
a98a4fb57f5 [ORC] Remove a stray decl that accidentally found its way in to r361322.
bb2b52769b4 Actaully lock accesses to OptionValueFileSpecList objects
dd0fe187ab8 Fix r361428 for Windows buildbots/mangling
e78cb1f20b2 Fix new enum-codegen.cpp test
00be4e68ad6 [docs] Make a note of the HowToUseLLJIT example in the ORCv2 design doc.
ed003679616 [Runtimes] If LLVM_INCLUDE_TESTS=On depend on gtest

Even with both gtest patches it still isn't working, ninja check-all is failing as before :(

[1100/1554] cd /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi-add-gtest/tools/clang/stage2-bins/runtimes/runtimes-bins && /usr/bin/cmake --build /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi-add-gtest/tools/clang/stage2-bins/runtimes/runtimes-bins/ --target check-runtimes --config RelWithDebInfo
ninja: error: '/home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi-add-gtest/tools/clang/stage2-bins/lib/libgtest.a', needed by 'compiler-rt/lib/asan/tests/ASAN_INST_TEST_OBJECTS.gtest-all.cc.x86_64-calls.o', missing and no known rule to make it
FAILED: runtimes/CMakeFiles/check-runtimes 
cd /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi-add-gtest/tools/clang/stage2-bins/runtimes/runtimes-bins && /usr/bin/cmake --build /home/wink/prgs/llvm/llvm-project/build-dist-use-lto-capable-linker-and-add-libcxxabi-add-gtest/tools/clang/stage2-bins/runtimes/runtimes-bins/ --target check-runtimes --config RelWithDebInfo

Is there a way to "force" gtest/gtest_main to get included just so I can see if there is any other check-all failures?

Other suggestions?

Even with both gtest patches it still isn't working, ninja check-all is failing as before :(

I wouldn't expect your gtest patch to resolve the issue. The CLANG_BOOTSTRAP_TARGETS variable is just a list of targets in the stage 2 build to generate wrapper targets in the stage 1 build for. It allows you to run ninja stage2-${target} instead of having to go into the stage2-bins directory and run ninja ${target}.

Is there a way to "force" gtest/gtest_main to get included just so I can see if there is any other check-all failures?

It isn't really about including the target. The issue is that there is a missing dependency. We need to make sure the gtest and gtest_main targets are built before the check-runtimes target (which is part of check-all.

Other suggestions?

My Linux test build is chugging along. It is going to take a while since it is in a VM, but once it completes I can look and see if I can work out why rL361436 didn't work to add the dependency.

beanz added a comment.May 23 2019, 1:23 PM

@winksaville I've figured out how to resolve the gtest issue, but unfortunately that isn't good enough to get the check-runtimes target working. A change went in back in February (rL353601), which breaks running compiler-rt's tests when building a distribution in non-trivial ways, which will unfortunately be difficult to resolve.

I will land the gtest fix sometime today, and I'm going to start working toward getting the larger issue resolved, although that is going to take some time.

@winksaville I've figured out how to resolve the gtest issue, but unfortunately that isn't good enough to get the check-runtimes target working. A change went in back in February (rL353601), which breaks running compiler-rt's tests when building a distribution in non-trivial ways, which will unfortunately be difficult to resolve.

I will land the gtest fix sometime today, and I'm going to start working toward getting the larger issue resolved, although that is going to take some time.

@beanz, glad we're making progress!

One other thing, my next goal, after we can successfully build and test the DistributionExample,
is to make DistributionExample_shared. I envision this creating clang that is dynamically linked
to libclang_shared.so and libLLVM.so and also provide the corresponding static libraries. Doing
this should provide Evangelos Foutras, whose is the maintainer of the llvm packages at Arch Linux,
an "easier" path to creating llvm Arch Linux packages that don't use BUILD_SHARED_LIBS.

Is this a reasonable goal and could DistrubutionExample_shared be added along side DistributionExample?

Added libcxxabi and rebased

winksaville added a comment.EditedMay 24 2019, 12:50 PM

So this compiles and improves on what we currently have and as such I think it should be
merged, although there maybe other solutions. It still fails ninja stage2-check-all with the
gtest problem which @beanz is working on resolving.

beanz accepted this revision.May 31 2019, 10:51 AM

This is good to land.

This revision is now accepted and ready to land.May 31 2019, 10:51 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2019, 10:33 AM