Page MenuHomePhabricator

[CMake] Support platform building builtins without a full toolchain
AbandonedPublic

Authored by beanz on Jan 27 2016, 3:22 PM.

Details

Summary

This patch adds support to building lib/builtins without a fully functioning toolchain. It allows you to bootstrap a cross-compiler, which previously couldn't be done with CMake.

This patch contains the following specific changes:

  • Split builtin-specific code out of config-ix.cmake into builtin-config-ix.cmake
  • Split some common CMake functionality needed by both builtins and sanitizers into base-config-ix.cmake
  • Moved some macros and functions out of config-ix.cmake into more common places
  • Added BuiltinTests.cmake which has some custom implementations of compiler testing that don't rely on having a functional liker or sysroot
  • Made lib/builtins/CMakeLists.txt able to be a top-level CMake configuration

I realize this patch is quite large, and I can break parts of it apart. I wanted to present it as a monolithic piece so that the other changes make sense.

I have tested this on Darwin targeting embedded Darwin, and on FreeBSD x86_64 targeting FreeBSD AArch64.

Diff Detail

Event Timeline

beanz updated this revision to Diff 46185.Jan 27 2016, 3:22 PM
beanz retitled this revision from to [CMake] Support platform building builtins without a full toolchain.
beanz updated this object.
beanz added reviewers: iains, samsonov, jroelofs.
beanz added a subscriber: llvm-commits.
samsonov edited edge metadata.Feb 5 2016, 1:58 PM

Thanks for doing this! Overall, I'm fine with the direction of this patch and it all makes sense.
I agree we have to introduce alternative and simple way to build "builtins" library for cross-compilation use cases w/o relying on valid working toolchain that is necessary for sanitizers.
Please do break this into a series of smaller patches: as I understand, many of them will just involve moving code around, splitting common configuration routines into separate files, etc.

Some issues/nits/suggestions that caught my eye as I skimmed through the patch.

CMakeLists.txt
270

Sink this include to the files which actually use smth. from AddLLVM? (test/CMakeLists.txt and unittest/CMakeLists.txt)?

cmake/Modules/BuiltinTests.cmake
2

This comment is wrong.

cmake/Modules/CompilerRTDarwinUtils.cmake
115

Why can't use the same TEST_COMPILE_ONLY variable as in test_target_arch, and merge this into darwin_test_archs instead?

cmake/base-config-ix.cmake
1

The comment is now irrelevant here: we actually set CMake version in a different place.

26

IIRC there were changes in these files recently: we've add COMPILER_RT_TEST_CXX_COMPILER, you might need to rebase.

74

Hm... do you think it's worth moving these architecture support checking to a yet another .cmake module?

84

Wait, are you using argstring at all in this branch?

cmake/config-ix.cmake
102

Looks like you've moved this macro to a different location, should it be deleted here now?

beanz added a comment.Feb 17 2016, 9:04 AM

This morning I committed the first two broken out patches in this sequence. They are both NFC, and I took into account the feedback provided by @samsonov.

The commits are r261105 and r261108.

The first commit pushes the AddLLVM dependency down into the test CMakeLists so it is only included where explicitly needed. The second commit moves the definitions of some macros out of config-ix.cmake into CompilerRTUtils.

beanz abandoned this revision.Apr 29 2016, 1:34 PM

Abandoning this review.

The last two parts of this change are out for review as:
http://reviews.llvm.org/D19692
http://reviews.llvm.org/D19742

Thanks,
-Chris