pirama 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 "")
----------------
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
```


https://reviews.llvm.org/D33561



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to