- User Since
- Aug 20 2014, 4:35 PM (247 w, 5 d)
I have a long story about this issue... Ask me about it sometime :).
@hintonda also, re-running the CMake command line and setting -D... is always a FORCE
@thakis Didn't we patch Ninja for this?
The CMake goop all looks reasonable to me. I think the new "correct" way to alter the compiler flags is add_compile_options, but since LLVM pretty heavily relies on modifying CMAKE_<LANG>_FLAGS it is probably better to be consistent than to start doing something new here.
Thank goodness @smeenai can spell because obviously I can't.
Looping in @EricWF.
A new take on how to do this, even more fun this time!
I just came up with a different and slightly crazier approach to this. I'm going to test it out and will upload a new version of this patch shortly.
Updates based on feedback from @smeenai
Updates based on review feedback.
Adding @zturner in case he has any thoughts.
@winksaville I just committed an update to the DistributionExamples a minute ago that should get them working as long as you aren't using a Darwin host. If you are using a Darwin host you need D62155 as well.
Sun, May 19
@winksaville, sorry those cache files pre-date the monorepo, and they haven’t been updated to support it. I will try and get a patch to update them today.
Sat, May 18
The main thing is to make sure that if you have proper integer sized members they are properly aligned and packed, which this seems to do well.
Fri, May 17
This is pretty cool. Since these are usually allocated as globals, this is saving pages of dirtied memory.
LGTM. Makes sense.
I would leave it out of any distribution (at least for now).
@hans I have some ideas on how to make package work. There is a simple solution to make it work as long as you're not using any runtime components, but I have an idea on how to make it work with runtime components.
Fixing the wording around LLVM_INSTALL_TOOLCHAIN_ONLY
Thu, May 16
LLVM generally has a policy of not leaving dead or unused code in tree. Changing to an error seems like the right approach given our coding guidelines.
Updates based on @winksaville's feedback
I don't know of anywhere that we use llvm_add_library to generate both static and shared libraries of the same code. The runtime libraries that are generated both ways roll their own solution to the problem.
I don't know that we want to support building component libraries as both STATIC and SHARED except in very specific and limited situations.
Adding information about distribution targets and LLVM_INSTALL_TOOLCHAIN_ONLY.
Why are the clang libraries being built STATIC and SHARED?
I will land this patch as-is today. I'll try and get a doc patch out for review today or tomorrow. @winksaville, I'm going to add a section to the document about building LLVM as a shared library for inclusion with tool distributions.
I should note, this and many more aspects of the build system were topics I discussed in a talk at the 2016 LLVM Dev meeting when we switched off autoconf:
There is a simpler example distribution configuration, but sadly there isn't documentation. That is something I can fix.
Wed, May 15
This patch doesn't actually include how the new code will be used, and the review doesn't include enough context for me to understand what is going on.
LLVM_BUILD_LLVM_DYLIB is actually not the important option to think about because it has no impact on this, rather LLVM_LINK_LLVM_DYLIB. That really depends on what tradeoffs you want. With that option off LLVM will be linked statically into the Clang library, which in the common case is fine because you can resolve all the LLVM & Clang APIs against it. That will be faster to load, but a larger binary distribution. With the option on, libclang_shared will link libLLVM, which will result in slower loading times, but smaller distributions. Both are workable situations.
Tue, May 14
Changed to lowercase 'c' to match other clang libraries.
@winksaville whether or not PIC is required for shared libraries varies by platform. These days LLVM defaults to -fPIC, and I'm not sure we actually support running LLVM without -fPIC on systems that require shared libraries to be PIC.
@winksaville that was my bad. I left off the new files in the commit because I forgot git-diff doesn't grab untracked files...
Mon, May 13
My change should not have decreased build time from trunk, nor should it have reduced the number of build steps. That patch should generate lib/libClang_shared.so, which will export the C++ API. libClang is unchanged since changing it would break existing clients of the library.
Sun, May 12
As an additional note, Arch linux should not be building clang with BUILD_SHARED_LIBS nor should it be distributing those ,so files. That isn't a supported configuration for Clang deployment.
I question the general utility of this patch, and I don't really think this does anything we want the build system doing.
Fri, May 10
Thu, May 9
Using a macro seems reasonable if a bit icky.
I think the two of you are speaking past each other with different goals.
Wed, May 8
Could you change the title and commit message to reference that this isn't just about using llvm_add_library it is more broadly about making the benchmark library behave like a proper LLVM library target when built in-tree.
Tue, May 7
Thu, May 2
Yep. This looks like the right solution. Un-used install destinations are effectively ignored by CMake, so there is no reason to selectively choose only one.
Libc++'s test cases aren't buildable without LLVM. Why should the benchmarks be?
Wed, May 1
That kinda makes it worse. We don't duplicate gtest in all the tools that use it, why are we duplicating this library?
@lebedev.ri I actually disagree with prefixing capabilities check variables. If other projects are checking the same thing you want them to use the same variable so that they don't all check the same thing over and over again.
Updating based on review feedback.
Tue, Apr 30
This all looks reasonable to me. It is kinda frustrating that CMake doesn't support any way to control the output name of an object file, or even the install name of one. I suspect that is probably because some of CMake's generation targets (like Xcode) don't give CMake explicit control over output object file names.
Fri, Apr 26
Seems reasonable to me.
Thu, Apr 25
Is there a specific version range of GCC that this miscompile occurs with?
Mon, Apr 22
I really like this. Using + to add variants seems much widely useful.
Apr 19 2019
Feb 20 2019
Feb 14 2019
Jan 31 2019
@phosek, does LLVM’s runtime directory do this correctly?
Jan 11 2019
Jan 7 2019
Dec 18 2018
It is expected that CMake will process all projects put under llvm/tools unless explicitly told not to. So, it is expected that it would process clang there. If you don't want it to process clang you need to set LLVM_TOOL_CLANG_BUILD=Off when configuring.
Dec 13 2018
Sorry for being behind on this, but I don't think this is the right solution to the problem.
Dec 4 2018
My $0.02 on the yaml discussion for tests:
From an Apple perspective this almost certainly needs @mtrent's eyes, as he owns our binary tools.
Nov 29 2018
How are you instructing lit to not run the tests that rely on these tools? Doesn't seem like you are, so I would expect check-llvm to fail on a clean build configuration due to missing tools.
Nov 16 2018
Nov 14 2018