-
Notifications
You must be signed in to change notification settings - Fork 16
Update CMake files to build on Mac using Homebrew packages. #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -34,3 +34,5 @@ endif() | |||
|
|||
set (CMAKE_CXX_FLAGS_DEBUG "-g3 -O0") | |||
set (CMAKE_CXX_FLAGS_RELEASE "-g -O2") | |||
set (CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fuse-ld=lld") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of lld
would require that LLVM from Homebrew is used by the host, correct? If so, this would prevent the compiler from being buildable with the native Apple developer toolchain. IMO, this is a major pain for Mac devs and thus something we should avoid to minimize friction for community contributors.
I'm somewhat confused why this is needed in the compiler core. Under what conditions is it needed? Is this only necessary when statically linking QASM into the compiler or is it needed when using QASM as a dylib too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The use of lld would require that LLVM from Homebrew is used by the host, correct? If so, this would prevent the compiler from being buildable with the native Apple developer toolchain.
Yes. The use of the LLVM Toolchain requires the use of the LLVM lld
linker. Not just on MacOS, but on Linux as well.
On MacOS, there are no compatibility guarantees between Apple's /usr/bin/ld
and LLVM's lld
.
On Linux, there are no compatibility guarantees between the GNU Binutils linkers (ld.bfd
and/or ld.gold
) and LLVM's lld
.
GCC and LLVM are two independent Toolchains, and they evolve independently.
This has been discussed at length in our weekly meetings and in Slack.
Apple's linker does not support init_priority
. Currently, it appears that it works for dynamic libraries because nothing crashes. That is not deterministic. Just because something doesn't crash, it doesn't mean it's a supported feature, or that it works. It could be silently - and subtly - broken. It could very well be a matter of sheer luck.
There is no indication, or documentation available, anywhere, that the MacOS linker supports init_priority
. init_priority
is a GNU extension, which was devised as a workaround for a C++ Standard Defect (see Static Initialization Fiasco
).
Apple no longer supports linking with archive libraries. Consequently, we do not, and cannot, support this feature either. We cannot support something that Apple does not support.
There is no compatibility guarantee between the Open Source LLVM/clang and Apple's LLVM/clang. Apple modifies their LLVM/clang and does not publish their source code changes, nor do they explain anywhere what changes they have made.
If you insist on linking with archive libraries, you must use the Open Source Homebrew LLVM toolchain, including the Open Source lld
linker.
If you do not link with archive libraries, compiling with the Open Source LLVM and linking with the Apple linker is an unsupported configuration. Just because it appears to work now, it does not mean it will continue to work in the future.
The linker depends on annotations and hints written into the object file by the compiler. We know nothing about the changes made by Apple to their LLVM/clang-based Toolchain. We do not know if they maintain compatibility with the Open Source LLVM lld
, or not.
You are welcome to try and experiment with any Toolchain configurations and combinations you wish. But the only supported configurations are:
- Open Source LLVM/clang Toolchain + Open Source LLVM
lld
, regardless of whether you link with archive libraries or dynamic libraries. On Apple, that is the only option - meaning Homebrew. - GNU Toolchain. That means GCC + either
ld.bfd
orld.gold
linkers. - Any other Toolchain combination is considered experimental and is usable at your own risk only.
A significant amount of work has been done by Kit and myself to make building on Apple easy and convenient for MacOS users. CMake will auto-detect at configure time the correct configuration for Apple builds.
The only task that is left up to the user is to install the list of Homebrew packages on their Mac.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Mac users will almost always have Homebrew installed, I propose making Homebrew's LLVM the supported version for qss-compiler
. If users choose to use Apple's LLVM/clang, all the power to them, but I don't think it makes sense for us--namely, Kevin, Stefan and Kit--to support this case since we have no control over Apple's proprietary toolchains.
My 2 cents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init_priority
is a GNU extension, which was devised as a workaround for a C++ Standard Defect (see Static Initialization Fiasco).
As an extension, init_priority
is not portable and so it's not a surprise that it breaks compatibility with Apple's toolchain. IMO, it's worth weighing the development effort required to simplify static initialization and ditch init_priority
in QASM against the consequences of sloughing off support for arguable one of the most common C++ toolchains.
Apple no longer supports linking with archive libraries. Consequently, we do not, and cannot, support this feature either. We cannot support something that Apple does not support.
Can you please add a reference for this claim? I believe the only "gotcha" with static linking on macOS is that it's not truly static, in that the final image will always require dynamic loading of Apple's frameworks etc.
Anyways. Proprietary or not, the Apple toolchain really ought to work. I'm not going to block this PR, but IMO we should make it a goal to re-enable it. And @kitbarton, we should add a check to this PR in the interim to fail and alert the user if AppleClang is detected.
Since Mac users will almost always have Homebrew installed
This is true, but they're unlikely to have LLVM installed, which on my system takes up 1.3 GB. They'll also need to set up the custom toolchain with their IDEs, which is rather annoying.
If users choose to use Apple's LLVM/clang, all the power to them
They won't be able to choose this after this PR. lld
won't be available, so linking will fail. This is what I take issue with here, mostly.
since we have no control over Apple's proprietary toolchains.
Proprietary or not, Apple's toolchain works for the vast majority of developers across a broad landscape of open source projects. The issue(s) here is that we're using a non-portable C++ extension to tip-toe around extensive use of a C++ feature wrought with design oversight (static initialization).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an extension, init_priority is not portable and so it's not a surprise that it breaks compatibility with Apple's toolchain.
I have alread explained what init_priority
is. If Apple chooses not to support it, that's Apple's problem.
Can you please add a reference for this claim?
Please examine Apple's Developer Guide, which is available online.
I believe the only "gotcha" with static linking on macOS is that it's not truly static, in that the final image will always require dynamic loading of Apple's frameworks etc.
I have no idea what this means. I don't know what a "static linking that's not truly static" means. We're not even talking about static linking here. We're talking about linking with archive libraries. That's a completely different thing than "static linking".
[ ... ] it's worth weighing the development effort required to simplify static initialization and ditch init_priority in QASM against the consequences of sloughing off support for arguable one of the most common C++ toolchains.
I welcome your patches that "simplify static initialization". Do you have a Pull Request I can look at?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- https://developer.apple.com/library/archive/qa/qa1118/_index.html
- https://developer.apple.com/forums/thread/706419
Please do some Google searching and familiarize yourselves with the differences between creating a dynamic executable by linking with one or more archive libraries (*.a) and static linking.
static linking has very little to do with archive libraries.
static linking is accomplished by passing -Bstatic
to the linker - if the linker supports this flag (ld.bfd
and ld.gold
do). lld
wants -static
, not -Bstatic
. No idea about Apple's linker.
The resulting output of static linking is a static executable.
At any rate, Apple doesn't provide the required crt[*].o's
for static linking, which makes static linking impossible on MacOS. More on that below.
static linking is not the same thing as creating a dynamic executable by linking one or more archive or shared libraries into it.
static linking requires - at a minimum - the presence of some specific crt[*].o
files to be distributed by the operating system. If these static linking specific files are not present, static linking is impossible. See MacOS and Linux.
Dynamic linking - which is the default on Linux and Darwin - also requires the presence of a complete set of crt[*].o
files. However, these files are different than the ones used in static linking, and these files are not interchangeable.
The second thing that is required for static linking and obtaining a statically linked executable is a special version of an archive Standard C Library [0]. That is, /usr/lib64/libc.a
on Linux, or libSystem.B.a
on MacOS. Good luck finding them. They are no longer being distributed; not by Linux and not by Darwin (MacOS) either.
About lld
's compatibility: compile qss-qasm
with the LLVM Toolchain - linking with the lld
linker - in debug mode (-O0 -g3) and then try to valgrind
it. That's on Linux, not MacOS. You will not be able to instrument a lld
linked executable with valgrind
. So much for the compatibility you read about online.
[0]. Depending on the operating system, and on how its Standard C Library is built and distributed, there may be some additional archive libraries that are required for static linking. A statically linked executable contains the interfaces between the kernel and the Standard C Library baked into it. I don't think I need to explain what a bad idea this is.
[1]. A statically linked executable cannot be compiled PIC/PIE. Not compiling PIC/PIE prevents ASLR. Thats a giant security hole.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have alread explained what
init_priority
is. If Apple chooses not to support it, that's Apple's problem.
As an extension, they don't have to support it. It's our problem if we choose to rely on an extension. It limits our toolchain support.
Apple no longer supports linking with archive libraries. Consequently, we do not, and cannot, support this feature either. We cannot support something that Apple does not support.
Circling back on this, Apple does support linking with archive libraries: https://developer.apple.com/forums/thread/706419?answerId=714469022#714469022
They just don't support static linking of the final image. Which we're not interested in anyways.
I have no idea what this means. I don't know what a "static linking that's not truly static" means. We're not even talking about static linking here. We're talking about linking with archive libraries. That's a completely different thing than "static linking".
Yup, you're right, that was a nonsensical statement, sorry for the confusion. What I meant was that Apple supports linking with archive libraries, but it doesn't support static linking of the final image.
Circling way back to my original PR comment:
Is this only necessary when statically linking QASM into the compiler or is it needed when using QASM as a dylib too? - Kevin
What I meant was, do we need to force lld
in qss-compiler when QASM is built as a dylib, or only if we're linking it in to qss-compiler via a library archive? My hunch is that we don't if it's a dylib, which I believe it would be on macOS after Kit's changes. If that's true, there's no need to require lld
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an extension, they don't have to support it. It's our problem if we choose to rely on an extension. It limits our toolchain support.
It's not an extension. It's a workaround for a Defect in the C++ Standard. All commercial compilers support it, including Microsoft's MSVC. That means: ICC (Intel compiler), AOCC (AMD compiler), armcc (ARM compiler), MSVC (as already stated), Sun CC (Sun/Oracle compiler). I believe IBM XLC supports it as well. Apple is the only outlier.
My hunch is that we don't if it's a dylib [ ... ]
Please do not rely on hunches. Hunches aren't defined anywhere.
I have already explained why we should not mix-and-match different Toolchains that make no explicit guarantee of compatibility.
Using the GNU Gold linker (ld.gold
) with Clang and obtaining binaries from Clang + ld.gold
that are 100% compatible with binaries from GCC + ld.gold
are two very different things.
If you really want to get into the nitty-gritty details, Open Source Clang is not 100% binary compatible with GCC. For example, Open Source Clang lies about its GCC compatibility version -- it's hardcoded to GCC 4.2.1. GCC 4.2.1 would not be able to compile C++17 or C11 or C18:
[steleman@pettyofficer][~/tmp][12/22/2022 18:14:28][1099]>> uname -a
Linux pettyofficer.stefanteleman.org 5.14.18-100.fc33.x86_64 #1 SMP Fri Nov 12 17:38:44 UTC2021 x86_64 x86_64 x86_64 GNU/Linux
[steleman@pettyofficer][~/tmp][12/22/2022 18:14:33][1100]>> clang --version
clang version 11.0.0 (Fedora 11.0.0-3.fc33)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
[steleman@pettyofficer][~/tmp][12/22/2022 18:14:35][1101]>> /usr/bin/clang -dM -E - < /dev/null | egrep GNU
#define __GNUC_MINOR__ 2
#define __GNUC_PATCHLEVEL__ 1
#define __GNUC_STDC_INLINE__ 1
#define __GNUC__ 4
[steleman@pettyofficer][~/tmp][12/22/2022 18:14:38][1102]>>
This is just one example. There are many others.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have addressed the question of linking with archive libraries on MacOS, and of the MacOS linker, several times, and at length.
If you are building on MacOS, you must use the Homebrew LLVM Toolchain. That requires using the Homebrew lld
linker.
If you are building on Linux, you can use either the GCC Toolchain, or the LLVM Toolchain provided by your Linux distro.
The only reason qss-compiler
has linked with archive libraries thus far is because of the original mis-configuration of the Conan build system. Conan was not configured to figure out how to properly create shared libraries on MacOS. This situation persisted for more than two years.
I added support for shared libraries on MacOS in qss-qasm
in commit 08f55a41babd4fa802d60d91f565368bcf357830.
Kit brought the qss-compiler
Conan build system to a working state.
The original mis-configuration of the Conan build system has now been cleared up and resolved. Linking with either archive or shared libraries works correctly on MacOS and on Linux, using the Toolchains specified in this PR.
You are free to use any other experimental Toolchains, or combinations thereof, at your own risk, and with no support.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not an extension. It's a workaround for a Defect in the C++ Standard.
It is an extension, workaround or not. We can avoid it by writing code that doesn't rely on a particular initialization order across TUs.
https://www.ibm.com/docs/fr/xcfbg/121.141?topic=extension-init-priority-variable-attribute-only
I have already explained why we should not mix-and-match different Toolchains that make no explicit guarantee of compatibility.
Regarding mixing and matching, the request for this is only relevant if QASM cannot be compiled with Apple's toolchain (which works for the rest of the stack). Which is not possible solely because QASM relies on static initialization order across TUs and Apple doesn't support the init_priority
extension.
(... also, if we're getting dylibs from Homebrew with these changes, those dylibs are almost certainly built with Apple's toolchain. So we're mixing and matching by compiling and linking qss-compiler
using LLVM from Homebrew against libs from Homebrew (e.g. GMP), yeah?)
I have addressed the question of linking with archive libraries on MacOS, and of the MacOS linker, several times, and at length.
If you are building on MacOS, you must use the Homebrew LLVM Toolchain. That requires using the Homebrew lld linker.
This is, from what I can find on Apple's developer site, not the case. Apple's toolchain supports linking with library archives.
https://developer.apple.com/forums/thread/706419?answerId=714469022#714469022
Conan was not configured to figure out how to properly create shared libraries on MacOS. This situation persisted for more than two years.
To my knowledge, none of our Conan configurations on macOS or otherwise will use shared libraries for everything, even with these changes. @kitbarton correct me if I'm wrong, but I'm under the impression that QASM and its dependencies (e.g. GMP) are the only libraries that become dylibs with your changes, and that this only happens on macOS.
Since this thread is getting rather long, here's a synopsis of where I stand:
- We should support Apple's toolchain. If not now, then later. To do this, we'd need to rewrite parts of QASM over time to eliminate reliance on static initialization order across TUs.
- I have no problem with using dynamic libs, but I'd like us to do so via Conan by upstreaming proper rpath fixes to packages we depend on.
- I have no problem with our CMake build system supporting dependencies coming from Conan or Homebrew, provided it is done cleanly. Thread here. But, if mixing and matching Apple toolchain built dylibs with Homebrew LLVM is an issue, then you're likely going to have a problem using Homebrew for dependencies.
@@ -11,5 +11,6 @@ set(LIBS | |||
qasm::qasm | |||
) | |||
target_link_libraries(QSSCAPI ${LIBS}) | |||
target_include_directories(QSSCAPI PUBLIC ${gmp_INCLUDES}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the right way to handle this would be in the custom Findgmp.cmake
for QASM. The qasm::qasm
target defined there should have its include directories set to include the GMP header dir. This way, target_link_libraries
used here should automatically include the necessary headers too. This is almost certainly what the Conan-generated Findgmp.cmake
is doing :).
Some useful details on how this works here: https://dominikberner.ch/cmake-interface-lib/
I'm going to abandon this PR for now. |
This patch contains a few updates to build qss-compiler on Mac: