Further improve LLVM building #18606

mppf · 2021-10-22T16:15:05Z

Continuing #18551

About static vs dynamic linking of LLVM libraries:

This PR leaves the situation that we were with respect to static vs dynamic linking on Cray systems. There, we link statically. I do not know if this is still required. See also PR Several build improvements for the bundled LLVM and Clang #11615 e334bb3 to link LLVM statically on Cray XC and PR Statically link in libstdc++ on crays for the compiler and llvm #3531 c4098c7 to link with libstdc++ statically on Cray XC.
On Mac OS X, LLVM is always statically linked, due to a behavior introduced in https://reviews.llvm.org/D13841 and brought up in Dynamic lib support for llvm? Homebrew/homebrew-core#47142 and llvm (all versions): add missing versioned libLLVM symlink; fix Mojave sysroot Homebrew/homebrew-core#47146 . It seems that this should be fixed in LLVM's CMake files that are creating the symbolic link on other systems.
At least on some linux systems, it appears that we weren't dynamically linking LLVM libraries (even though we intended to). I think this might be related to unnecessary quoting that we had on the cmake command to configure LLVM.

This PR addresses #18589 by adjusting the chpl link to use -lclang-cpp (which is a dynamic library) instead of several static libraries when using CHPL_LLVM=system. It adjusts chpl to always link statically with LLVM and Clang libraries when using CHPL_LLVM=bundled.

Additionally, it handles a few follow-ups to #18551 including removing code that is not needed and addressing an issue with gasnet-mpi configurations with a custom GCC.

About the presence of -clang-cpp on various system-installs of LLVM:

It is present on:
- CentOS 8 (LLVM 11 .so provided by clang-devel)
- Fedora 34 (but that system uses LLVM 12)
- Ubuntu 21.10
- Debian 11 Bullseye which has LLVM 11
- FreeBSD 13
- FreeBSD 12.1
It is not present on these systems that only have older LLVM versions:
- Debian 9 Stretch (only has LLVM 7)
- Debian 10 Buster (only has LLVM 7)

Future work:

Address rpath issues with make install -- https://github.com/Cray/chapel-private/issues/2629

Reviewed by @ronawho - thanks!

And also adjust makefiles to remove cases for LLVM 9/10 now as 11 is the current requirement. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Need to check that this works OK -- e334bb3 enabled this logic and said "Otherwise if the PrgEnv module isn't loaded, problems finding libstdc++." If we can't use the dylib on Crays then we will need to put back in the static library link strings for the clang libraries. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Removes util/config/gather-cray-prgenv-arguments.bash util/config/gather-pe-chapel-pkgconfig-libs.bash and makes adjustments to Makefiles and chplenv scripts to allow that. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

I haven't seen a problem with it yet but I don't want to get in to changing it in this PR. See PR chapel-lang#3531 and PR chapel-lang#11615 for some history in this area. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Only does something with e.g. make CHPL_LLVM_DYNAMIC=0 --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

to address failures with gasnet-mpi configurations --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

mppf · 2021-10-27T22:59:25Z

third-party/llvm/Makefile.include-bundled

@@ -1,5 +1,12 @@
 include $(THIRD_PARTY_DIR)/llvm/Makefile.share-bundled

+# Link statically on Cray systems to avoid problems with
+# finding libstdc++ with different PrgEnv compilers.
+CHPL_LLVM_DYNAMIC := 1


Question for reviewer: Given that we weren't linking dynamically before and such linking can lead to rpath issues, should we switch to always statically linking with the bundled LLVM for now until the rpath issues are resolved?

I would be fine sticking with static linking for the bundled version.

OK, that's what I've done

ronawho · 2021-10-28T16:01:49Z

runtime/src/comm/ugni/Makefile.share

-    endif
+    # these should already be gathered in CHPL_LLVM_CLANG_COMPILE_ARGS
+    # by printchplenv.
+    CHPL_UGNI_PRGENV_CFLAGS := $(CHPL_MAKE_LLVM_CLANG_COMPILE_ARGS)


Do we still need the above comment and conditionals or can we just always set this (wondering if CHPL_MAKE_LLVM_CLANG_COMPILE_ARGS is empty when the above conditions aren't set)

Yeah, I think we could always set it and I'd expect it to be empty if the above conditions aren't set. For now though I will leave this as-is in the PR.

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

ronawho · 2021-10-28T18:51:02Z

I thought I left a comment along this lines here, but I'm not seeing it now -- How often do you expect we'll need to make change like this to adapt to new llvm versions or different packaging on different OS's? Do you think it'd be worthwhile to add testing on a bunch of different OS's with a system llvm? (Either through automated vagrant testing or fixed VMs?)

mppf · 2021-10-28T18:55:17Z

In this particular case, we're adapting to what the LLVM project recommends (at least, according to https://archives.gentoo.org/gentoo-dev/message/6d3cf88d858fcf3fbb11818ce5d6ea42 ). I don't expect we'll need a lot of changes like this for different OSes. However I think it would absolutely be worthwhile to add testing for a bunch of different OSes with system LLVM. For that I am hoping we use vagrant or docker or singularity or something along those lines. (I have done some testing with vagrant stuff we have on different OSes with system LLVMs, but it's more spot-checking). I prefer these approaches to fixed VMs because the OSes to test change over time as some of them become end-of-life'd.

ronawho · 2021-10-28T19:07:16Z

However I think it would absolutely be worthwhile to add testing for a bunch of different OSes with system LLVM. For that I am hoping we use vagrant or docker or singularity or something along those lines. (I have done some testing with vagrant stuff we have on different OSes with system LLVMs, but it's more spot-checking). I prefer these approaches to fixed VMs because the OSes to test change over time as some of them become end-of-life'd.

👍 Can we capture this on an existing vagrant issue or create a new one to capture it?

@ronawho

Improve LLVM configuration on Mac OS X Follow-up to PR #18606 * Add isysroot argument even for system llvm * This appears to be necessary on Mac OS X 11.6.1 with Homebrew llvm version 11.1.0. * Link clang statically for Mac OS X with system LLVM * note that we were already linking statically with the LLVM binaries, but previously were linking clang dynamically * intended to resolve problems building mason (Cray/chapel-private#2702) * Add -I and -L paths according to sysroot * The `-L` path at least seems to be necessary on Mac OS 11.6 and newer. See https://developer.apple.com/forums/thread/666700 for some background in this area. Even though the `-isysroot` path covers it, the linking call does not seem to use it. - [x] homebrew system LLVM 11.1 functions on Big Sur - [x] homebrew system LLVM 11.1 functions on Catalina - [x] homebrew system LLVM functions on Mojave - [x] bundled LLVM functions on Big Sur and Catalina - [x] full local testing Reviewed by @ronawho - thanks!

mppf added 6 commits October 23, 2021 11:33

Use dylib clang library instead of static ones

f58486e

And also adjust makefiles to remove cases for LLVM 9/10 now as 11 is the current requirement. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Add reminder TODO comment

38ff145

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Update prereqs to include libclang-cpp11-dev on Ubuntu

8d3ee7e

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Update CI prereqs to include libclang-cpp11-dev

8658db1

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

mppf force-pushed the llvm-dylib branch from 6d60565 to 8658db1 Compare October 23, 2021 15:33

mppf added 6 commits October 26, 2021 09:58

Adjust FreeBSD command

794bf5f

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Remove unnecessary quotes

4b1aac4

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Add stub to dynamically link with CHPL_LLVM=system

64c706e

Only does something with e.g. make CHPL_LLVM_DYNAMIC=0 --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Remove maybeRelaunchInPrgEnv

9bad31f

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

Don't use clang args for override ld

8c684ff

to address failures with gasnet-mpi configurations --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

mppf requested a review from ronawho October 27, 2021 22:54

mppf commented Oct 27, 2021

View reviewed changes

ronawho reviewed Oct 28, 2021

View reviewed changes

ronawho approved these changes Oct 28, 2021

View reviewed changes

Statically link bundled LLVM/clang for now

7f4799e

--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>

ronawho approved these changes Oct 28, 2021

View reviewed changes

mppf merged commit d8a0fe5 into chapel-lang:main Oct 28, 2021

mppf deleted the llvm-dylib branch October 28, 2021 19:57

mppf mentioned this pull request Oct 28, 2021

Unable to build Chapel 1.25.0 under Gentoo Linux with CHPL_LLVM=system #18589

Closed

mppf mentioned this pull request Nov 16, 2021

Improve LLVM configuration on Mac OS X #18727

Merged

5 tasks

mppf mentioned this pull request Feb 18, 2022

Fix problem with static link order with static clang and llvm #19272

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Further improve LLVM building #18606

Further improve LLVM building #18606

mppf commented Oct 22, 2021 •

edited

Loading

mppf Oct 27, 2021

ronawho Oct 28, 2021

mppf Oct 28, 2021

ronawho Oct 28, 2021

mppf Oct 28, 2021

ronawho commented Oct 28, 2021

mppf commented Oct 28, 2021

ronawho commented Oct 28, 2021

Further improve LLVM building #18606

Further improve LLVM building #18606

Conversation

mppf commented Oct 22, 2021 • edited Loading

mppf Oct 27, 2021

Choose a reason for hiding this comment

ronawho Oct 28, 2021

Choose a reason for hiding this comment

mppf Oct 28, 2021

Choose a reason for hiding this comment

ronawho Oct 28, 2021

Choose a reason for hiding this comment

mppf Oct 28, 2021

Choose a reason for hiding this comment

ronawho commented Oct 28, 2021

mppf commented Oct 28, 2021

ronawho commented Oct 28, 2021

mppf commented Oct 22, 2021 •

edited

Loading