[PATCH] D11082: [CMake] Adding some utility functions for Darwin builds into a new CompilerRTDarwinUtils.cmake module

Alexey Samsonov vonosmas at gmail.com
Wed Jul 22 21:03:40 PDT 2015


samsonov added inline comments.

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:26
@@ +25,3 @@
+# link for.
+function(darwin_get_toolchain_supported_archs output_var)
+  execute_process(
----------------
beanz wrote:
> samsonov wrote:
> > beanz wrote:
> > > samsonov wrote:
> > > > Looks like this should be the Darwin version of the complex code that generates `COMPILER_RT_SUPPORTED_ARCH`. Can you just call it from there?
> > > Hooking this into where we setup `COMPILER_RT_SUPPORTED_ARCH` would be pretty complicated. Much of that code in config-ix.cmake is based around the idea of building compiler-rt for a single target. Also `darwin_get_toolchain_supported_archs` returns a long list of architectures we don't care about. On the latest Xcode the list is: `armv6 armv7 armv7s arm64 i386 x86_64 x86_64h armv6m armv7m armv7em`.
> > > 
> > > For Darwin we currently build for //n// architectures and //m// platforms generating an //n// x //m// configuration matrix. To take the architectures returned by `ld -v` and group them into platform-architecture pairs we'd also need all the compiler flags properly setup for each platform. Currently that is all done in `CMakeLists.txt:272-315` well after config-ix is processed.
> > > 
> > > In order to do this work in config-ix, we would need to shift all of that around, which would make these patches quite substantial. I'd like to avoid doing that because I really want to get rid of all of that, but I don't yet have a sane transition strategy yet.
> > > 
> > > Thoughts?
> > Yes, I agree this would be complicated, but we'll have to do it anyway =/ I don't think that landing this patch before the rework would make things easier for us, the contrary is more likely.
> > 
> > The code in `config-ix.cmake` is actually based on the idea that we target a single platform/OS, and (possibly) several architectures. This is the "usual" case for 64-bit Linux, where we build runtimes for both x86_64 and i386. But this is `n x 1` as opposed to `n x m` case on Mac. I think we should rework the code assuming that we build runtimes for "triples" instead (well, in our case that would be OS/arch pairs).
> > 
> > Now determining compile flags for target is already split between `config-ix.cmake` (`TARGET_${arch}_FLAGS`) and `CMakeLists.txt` (`DARWIN_${os}_CFLAGS`), it would be better to consolidate this part before moving forward. I have no transition strategy yet either, but can try to up-prioritize working on that.
> > 
> > If you want to proceed with this patch as quickly as possible anyway, I would still suggest to move the flag setup logic to config-ix, under
> >   if("${LLVM_NATIVE_ARCH}" STREQUAL "X86")
> >     if(APPLE)
> > , as we don't care about building ASan on other Mac OS hosts, do we?  
> > 
> > 
> > 
> I actually don't think that the code I'm proposing in this patch should exist in the "fixed" compiler-rt build scripts. I see it as a temporary solution to get the CMake build system to be feature-for-feature compatible in open-source with the things we use autoconf for at Apple.
> 
> With Xcode 7 we have shipped asan support for iOS, so we need CMake to support it.
> 
> I actually think the Linux cmake code as it is today is MUCH closer to what we actually want than the Darwin code, so I'd really prefer not to hack bad behaviors into the existing linux code.
> 
> Once I've gotten the feature parity between CMake and Autoconf, the next step I want to take is to start putting in a new behavior for how CMake builds on Darwin (controlled by a variable), which will allow it to build for a single platform at a time. I expect that change will basically be following the Linux code paths with very few exceptions.
> 
> I know this really isn't a good justification because I'm trying to convince you to be ok with something that I think is a bad idea, but this is a temporary solution.
> 
> Does that make sense?
Oh, I would love to have better support for CMake on Darwin, so that it would (finally) replace autoconf build, which keeps causing problems for us. Do you think you will be able to drop autoconf build soon after adding support for ios/iossim?

Thanks for clarification regarding your future plans. I'm actually in favor of this idea (building for one platform at a time, and having separate build trees for different OS). We do smth. similar for Android already.

I'm not convinced that it would be easier to land current patches, and then rework them, instead of building OS-specific trees upfront. I would agree on that only if you will delete the autoconf build system right after landing this patch set, so that we at least keep the level of horridness in the build system code :)


http://reviews.llvm.org/D11082







More information about the llvm-commits mailing list