This is an archive of the discontinued LLVM Phabricator instance.

[CMake] Add Android toolchain CMake cache files.
ClosedPublic

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

Details

Event Timeline

aoli created this revision.May 25 2017, 12:35 PM
aoli updated this revision to Diff 100282.May 25 2017, 12:36 PM

Fix wrong flags.

what about the builtins?

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

Add builtin configurations.

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 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
27

Same comment as ASSERTIONS below.

37

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
37

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
37

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
37

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.

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.

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

Remove test code. Sorry for the noise.

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