jroelofs added inline comments.
================
Comment at: cmake/caches/Android-stage2.cmake:37
+ 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 "")
----------------
pirama wrote:
> jroelofs wrote:
> > pirama wrote:
> > > 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).
> > 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.
> 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
> ```
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.
https://reviews.llvm.org/D33561
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
