Improve LLVM configuration on Mac OS X #18727
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This appears to be necessary on Mac OS X 11.6.1 with Homebrew llvm version 11.1.0. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
--- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
This 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. --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
|
I verified that this resolves https://github.com/Cray/chapel-private/issues/2702 in my setup (Mojave, CHPL_LLVM=system, homebrew llvm). |
ronawho
reviewed
Nov 16, 2021
| @@ -324,7 +324,8 @@ def get_sysroot_resource_dir_args(): | |||
| args = [ ] | |||
| target_platform = chpl_platform.get('target') | |||
| llvm_val = get() | |||
| if target_platform == "darwin" and llvm_val == "bundled": | |||
| if (target_platform == "darwin" and | |||
| (llvm_val == "bundled" or llvm_val == "system")): | |||
There was a problem hiding this comment.
I don't think it's worth changing, but just checking for my sake -- is this the same as llvm_val != 'none'?
There was a problem hiding this comment.
Not quite, because we also can have 'unset' as the CHPL_LLVM value.
ronawho
approved these changes
Nov 16, 2021
7 tasks
mppf
added a commit
to mppf/chapel
that referenced
this pull request
Nov 22, 2021
* Don't add sysroot args for system LLVM on Mac OS X * Don't add -I and -L equivalents to sysroot argument. I think these are both not necessary. I think that these were workarounds for a problem local to my system (because things started to behave better when I reinstalled CommandLineTools). --- Signed-off-by: Michael Ferguson <mppf@users.noreply.github.com>
mppf
added a commit
that referenced
this pull request
Nov 29, 2021
Address command quoting issue in printchplenv For #18741 This PR adjusts chpl_compiler.get_compiler_command and chpl_llvm.get_llvm_clang to return a Python list so that it is easier to construct the correctly quoted command to run when using these. It also adjusts get_compiler_version to accept flag of host or target. While there, avoid adding a gcc prefix argument for clang when the prefix is just the normal /usr. It removes two workarounds I added in PR #18727 because reinstalling CommandLineTools fixed the problem for me and these workarounds weren't sufficient to get the third-party code compiling. Future Work: * figure out how to resolve linker errors like `object file ... was built for newer macOS version (11.6) than being linked (11.0)` with CHPL_LLVM=bundled when linking `chpl` itself. So far these appear to be harmless. * Look into problems with Mojave with CHPL_LLVM=bundled (wasn't working for @e-kayrakli but so far we are not sure if that is a local issue) Verified that `make check` passes on - [x] Big Sur with CHPL_LLVM=system with Homebrew LLVM 11.1 - [x] Big Sur with CHPL_LLVM=bundled - [x] Mojave with CHPL_LLVM=system with Homebrew LLVM 10.1 - [x] Catalina with CHPL_LLVM=system with Homebrew LLVM 10.1 - [x] Ubuntu builds with CHPL_LLVM=bundled, CHPL_TARGET_COMPILER=clang, custom GCC, and no clang in path - [x] Ubuntu builds with CHPL_LLVM=system And - [x] full local testing Reviewed by @e-kayrakli - thanks!
5 tasks
mppf
added a commit
that referenced
this pull request
Feb 22, 2022
Fix problem with static link order with static clang and llvm This resolves a problem on CentOS 7 which has an error like this: ``` : CommandLine Error: Option 'enable-vfe' registered more than once! LLVM ERROR: inconsistency in registered CommandLine options ``` We have some evidence that this error was caused by statically linking LLVM libraries but dynamically linking clang-cpp. For one thing, on that system, changing to statically linking both solved this issue. This PR changes chpl_llvm.py to ask `llvm-config` if it has shared libraries available (with `llvm-config --shared-mode`) and if not it will use the static linking approach for both the LLVM and clang dependencies. Additionally, when using static linking for LLVM and clang, on CentOS 7, the linker was picky about the order. Since the link line should have the things depended upon later, this PR puts the `-l` flags for clang libraries before the `-l` flags for LLVM libraries. On Mac OS X with Homebrew LLVM, we were using the same (arguably incorrect) `-l` ordering but for some reason the linker there did not complain. Note that we statically link both LLVM and clang on Mac OS X (since PR #18727). This PR leaves it this way. Historically, there have been problems with upstream LLVM and Homebrew in building a dynamic library that works with `llvm-config` (in particular the versioned file, like `libLLVM-11.dylib`, is missing). See [this LLVM bug report](llvm/llvm-project#39599). I expect that we will be able to allow dynamic linking on Mac OS X once we are able to establish that the issue is resolved. We are statically linking with CHPL_LLVM=bundled and this PR does not change that. Reviewed by @Maxrimus - thanks! - [x] 'hello' builds and runs with system LLVM on Ubuntu 21 - [x] 'hello' builds and runs with system LLVM on SLES 12 - [x] 'hello' builds and runs with system LLVM on Mac OS X with Homebrew - [x] 'hello' builds and runs with bundled LLVM on Ubuntu 21 - [x] full local testing
Closed
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to PR #18606
Add isysroot argument even for system llvm
version 11.1.0.
Link clang statically for Mac OS X with system LLVM
but previously were linking clang dynamically
(https://github.com/Cray/chapel-private/issues/2702)
Add -I and -L paths according to sysroot
-Lpath at least seems to be necessary on Mac OS 11.6 andnewer. See https://developer.apple.com/forums/thread/666700 for some
background in this area. Even though the
-isysrootpath covers it,the linking call does not seem to use it.
Reviewed by @ronawho - thanks!