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

Alexey Samsonov vonosmas at gmail.com
Tue Jul 21 18:03:23 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:
> > 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?  





http://reviews.llvm.org/D11082







More information about the llvm-commits mailing list