This is an archive of the discontinued LLVM Phabricator instance.

Differential D33561

[CMake] Add Android toolchain CMake cache files.
ClosedPublic

Authored by aoli on May 25 2017, 12:35 PM.

Details

Reviewers
beanz
srhines
Commits
rG42698d09b80a: [CMake] Add Android toolchain CMake cache files.
rC309407: [CMake] Add Android toolchain CMake cache files.
rL309407: [CMake] Add Android toolchain CMake cache files.
Summary

Thoes files are used to build Android toolchain. D32816 makes it possible to build runtimes for targets.

Diff Detail

Event Timeline

aoli created this revision.May 25 2017, 12:35 PM
Harbormaster completed remote builds in B6765: Diff 100281.May 25 2017, 12:35 PM
Herald added a subscriber: mgorny. · View Herald TranscriptMay 25 2017, 12:35 PM
aoli updated this revision to Diff 100282.May 25 2017, 12:36 PM

Fix wrong flags.

jroelofs added a subscriber: jroelofs.May 25 2017, 12:45 PM

what about the builtins?

aoli updated this revision to Diff 100308.May 25 2017, 3:12 PM

Add builtin configurations.

Harbormaster completed remote builds in B6774: Diff 100308.May 25 2017, 3:12 PM
aoli added a comment.May 25 2017, 3:14 PM

@jroelofs we are not building builtins currently but we are planning to do so. I just added some basic configurations for builtins.

aoli added a subscriber: pirama.Jun 2 2017, 10:49 AM
aoli added a parent revision: D32816: [CMake] Support multi-target runtimes build.Jul 6 2017, 3:46 PM
aoli updated this revision to Diff 107158.Jul 18 2017, 12:44 PM

Add more configs.

pirama added inline comments.Jul 18 2017, 12:52 PM
cmake/caches/Android-stage2.cmake
28

Same comment as ASSERTIONS below.

38

Should we initialize it to LLVM_ENABLE_ASSERTIONS rather than defaulting to ON? So assertions can be enabled/disabled for all targets with just one switch (and then over-ride with per-target flags, if necessary).

jroelofs added inline comments.Jul 18 2017, 1:00 PM
cmake/caches/Android-stage2.cmake
38

No. Absolutely not. Whether or not the bits of the toolchain that run on the host have assertions should be *completely* independent of whether the target bits do or don't. Do not conflate the host with the target.

pirama added inline comments.Jul 18 2017, 1:15 PM
cmake/caches/Android-stage2.cmake
38

Maybe I wasn't clear. I suggest that we don't set RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS to ON here and rather initialize it based on another flag. I was incorrect to suggest we reuse LLVM_ENABLE_ASSERTIONS. We should rather have something like ANDROID_RUNTIMES_ENABLE_ASSERTIONS.

The motivation is to reduce the number of additional flags/switches, under the assumption that assert-enabled or assert-disabled builds of the runtimes are generated simultaneously.

To generate assert-enabled builds, the invocation would be:

$ cmake ... -DANDROID_RUNTIMES_ENABLE_ASSERTIONS=ON

instead of

$ cmake ... -DRUNTIMES_armv7-linux-android_LLVM_ENABLE_ASSERTIONS=ON -DRUNTIMES-aarch64-linux-android_LLVM_ENABLE_ASSERTIONS=ON ...

If I understand CMake correctly, we can still enable assertions only for a particular target by:

$ cmake ...  -DANDROID_RUNTIMES_ENABLE_ASSERTIONS=OFF -DRUNTIMES-aarch64-linux-android_LLVM_ENABLE_ASSERTIONS=ON
jroelofs added inline comments.Jul 18 2017, 1:40 PM
cmake/caches/Android-stage2.cmake
38

Yeah, collecting them under a common default seems fine. The only thing I was objecting to was having that default be tied to the settings of the host part of the build.

aoli updated this revision to Diff 107951.Jul 24 2017, 1:18 PM

Add STAGE2_ prefix to pass stage2 variables. Add ANDROID_RUNTIMES_BUILD_TYPE, ANDROID_RUNTIMES_ENABLE_ASSERTIONS, and ANDROID_BUILTINS_BUILD_TYPE.

Harbormaster completed remote builds in B8536: Diff 107951.Jul 24 2017, 1:18 PM
aoli added a reviewer: beanz.Jul 24 2017, 1:20 PM
aoli updated this revision to Diff 107953.Jul 24 2017, 1:22 PM

Remove test code.

Harbormaster completed remote builds in B8537: Diff 107953.Jul 24 2017, 1:22 PM
aoli updated this revision to Diff 107954.Jul 24 2017, 1:24 PM

Remove test code. Sorry for the noise.

Harbormaster completed remote builds in B8538: Diff 107954.Jul 24 2017, 1:24 PM
srhines accepted this revision.Jul 27 2017, 11:49 AM

Awesome!

This revision is now accepted and ready to land.Jul 27 2017, 11:49 AM
aoli closed this revision.Jul 28 2017, 10:41 AM

Revision Contents

PathSize
cmake/
caches/
30 lines
31 lines

Diff 100308

cmake/caches/Android-stage2.cmake

  • This file was added.
set(LLVM_TARGETS_TO_BUILD X86;ARM;AArch64 CACHE STRING "")
set(CLANG_VENDOR Android CACHE STRING "")
set(CMAKE_BUILT_TYPE RELEASE CACHE STRING "")
set(LLVM_ENABLE_THREADS OFF CACHE BOOL "")
set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
set(LLVM_BUILTIN_TARGETS "i686-linux-android;x86_64-linux-android;aarch64-linux-android;armv7-linux-android" CACHE STRING "")
foreach(target i686;x86_64;aarch64;armv7)
set(BUILTINS_${target}-linux-android_ANDROID 1 CACHE STRING "")
set(BUILTINS_${target}-linux-android_CMAKE_ASM_FLAGS ${Android_${target}_C_FLAGS} CACHE PATH "")
set(BUILTINS_${target}-linux-android_CMAKE_C_FLAGS ${Android_${target}_C_FLAGS} CACHE PATH "")
set(BUILTINS_${target}-linux-android_CMAKE_SYSROOT ${Android_${target}_SYSROOT} CACHE PATH "")
endforeach()
set(LLVM_RUNTIME_TARGETS "i686-linux-android;x86_64-linux-android;aarch64-linux-android;armv7-linux-android" CACHE STRING "")
foreach(target i686;x86_64;aarch64;armv7)
set(RUNTIMES_${target}-linux-android_ANDROID 1 CACHE STRING "")
set(RUNTIMES_${target}-linux-android_CMAKE_ASM_FLAGS ${Android_${target}_C_FLAGS} CACHE PATH "")
set(RUNTIMES_${target}-linux-android_CMAKE_C_FLAGS ${Android_${target}_C_FLAGS} CACHE PATH "")
set(RUNTIMES_${target}-linux-android_CMAKE_CXX_FLAGS ${Android_${target}_CXX_FLAGS} CACHE PATH "")
set(RUNTIMES_${target}-linux-android_CMAKE_SYSROOT ${Android_${target}_SYSROOT} CACHE PATH "")
set(RUNTIMES_${target}-linux-android_COMPILER_RT_ENABLE_WERROR ON CACHE BOOL "")
set(RUNTIMES_${target}-linux-android_COMPILER_RT_INCLUDE_TESTS OFF CACHE BOOL "")
set(RUNTIMES_${target}-linux-android_LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
set(RUNTIMES_${target}-linux-android_LLVM_ENABLE_THREADS OFF CACHE BOOL "")
piramaUnsubmitted
Not Done
ReplyInline Actions

Same comment as ASSERTIONS below.

pirama: Same comment as ASSERTIONS below.
set(RUNTIMES_${target}-linux-android_LLVM_INCLUDE_TESTS OFF CACHE BOOL "")
endforeach()
piramaUnsubmitted
Not Done
ReplyInline Actions

Should we initialize it to LLVM_ENABLE_ASSERTIONS rather than defaulting to ON? So assertions can be enabled/disabled for all targets with just one switch (and then over-ride with per-target flags, if necessary).

pirama: Should we initialize it to LLVM_ENABLE_ASSERTIONS rather than defaulting to ON? So assertions…
jroelofsUnsubmitted
Not Done
ReplyInline Actions

No. Absolutely not. Whether or not the bits of the toolchain that run on the host have assertions should be *completely* independent of whether the target bits do or don't. Do not conflate the host with the target.

jroelofs: No. Absolutely not. Whether or not the bits of the toolchain that run on the host have…
piramaUnsubmitted
Not Done
ReplyInline Actions

Maybe I wasn't clear. I suggest that we don't set RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS to ON here and rather initialize it based on another flag. I was incorrect to suggest we reuse LLVM_ENABLE_ASSERTIONS. We should rather have something like ANDROID_RUNTIMES_ENABLE_ASSERTIONS.

The motivation is to reduce the number of additional flags/switches, under the assumption that assert-enabled or assert-disabled builds of the runtimes are generated simultaneously.

To generate assert-enabled builds, the invocation would be:

$ cmake ... -DANDROID_RUNTIMES_ENABLE_ASSERTIONS=ON

instead of

$ cmake ... -DRUNTIMES_armv7-linux-android_LLVM_ENABLE_ASSERTIONS=ON -DRUNTIMES-aarch64-linux-android_LLVM_ENABLE_ASSERTIONS=ON ...

If I understand CMake correctly, we can still enable assertions only for a particular target by:

$ cmake ...  -DANDROID_RUNTIMES_ENABLE_ASSERTIONS=OFF -DRUNTIMES-aarch64-linux-android_LLVM_ENABLE_ASSERTIONS=ON
pirama: Maybe I wasn't clear. I suggest that we don't set RUNTIMES_${target}_LLVM_ENABLE_ASSERTIONS to…
jroelofsUnsubmitted
Not Done
ReplyInline Actions

Yeah, collecting them under a common default seems fine. The only thing I was objecting to was having that default be tied to the settings of the host part of the build.

jroelofs: Yeah, collecting them under a common default seems fine. The only thing I was objecting to was…

cmake/caches/Android.cmake

  • This file was added.
# This file sets up a CMakeCache for an Android toolchain build.
set(LLVM_TARGETS_TO_BUILD X86 CACHE STRING "")
set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
set(CMAKE_BUILD_TYPE RELEASE CACHE STRING "")
set(LLVM_BUILD_TOOLS OFF CACHE BOOL "")
set(LLVM_BUILD_RUNTIME OFF CACHE BOOL "")
set(LLVM_ENABLE_ASSERTIONS ON CACHE BOOL "")
set(LLVM_ENABLE_THREADS OFF CACHE BOOL "")
set(LLVM_TOOL_CLANG_TOOLS_EXTRA_BUILD OFF CACHE BOOL "")
set(LLVM_TOOL_OPENMP_BUILD OFF CACHE BOOL "")
set(CLANG_VENDOR Android CACHE STRING "")
foreach(target i686;x86_64;aarch64;armv7)
if (Android_${target}_C_FLAGS)
list(APPEND EXTRA_ARGS -DAndroid_${target}_C_FLAGS=${Android_${target}_C_FLAGS})
endif()
if (Android_${target}_SYSROOT)
list(APPEND EXTRA_ARGS -DAndroid_${target}_SYSROOT=${Android_${target}_SYSROOT})
endif()
if (Android_${target}_CXX_FLAGS)
list(APPEND EXTRA_ARGS -DAndroid_${target}_CXX_FLAGS=${Android_${target}_CXX_FLAGS})
endif()
endforeach()
set(CLANG_ENABLE_BOOTSTRAP ON CACHE BOOL "")
set(CLANG_BOOTSTRAP_CMAKE_ARGS "${EXTRA_ARGS} -C${CMAKE_CURRENT_LIST_DIR}/Android-stage2.cmake" CACHE STRING "")