[PATCH] D58951: [compiler-rt][tests] Improve handling with non-default toolchains

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

If I've understood this correctly the patch introduces COMPILER_RT_TOOLCHAIN_CFLAGS which we don't expect users to set directly, but instead set -gcc-toolchain if the compiler is clang and CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN is set?

My understanding is that CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN is not respected when cross-compiling compiler-rt for Arm and AArch64. I've always had to pass in --gcc-toolchain manually in the C flags. Is there any reason why we should make a distinction between a native and a cross-compile here? I'd say we were more likely to want --gcc-toolchain when cross-compiling. If that is the case it would be better to find a way of using the existing COMPILER_RT_TEST_CFLAGS? This would also make it a bit less confusing as COMPILER_RT_TOOLCHAIN_CFLAGS is only used in the context of tests and COMPILER_RT_TEST_CFLAGS is where I would expect to see that being used.

> An issue encountered when preparing this change was that COMPILER_RT_UNITTEST_LINK_FLAGS is dropped in many places, unlike COMPILER_RT_UNITTEST_CFLAGS. This patch also attempts to remove that inconsistency.

Personally I'd recommend splitting that part out into a separate patch, it will make it a bit easier to untangle which parts of the patch are concerned with the toolchain changes.



================
Comment at: cmake/config-ix.cmake:192
 macro(get_test_cc_for_arch arch cc_out cflags_out)
   if(ANDROID OR ${arch} MATCHES "arm|aarch64")
     # This is only true if we are cross-compiling.
----------------
At a first glance this doesn't look right in the context of a native arm or aarch64 build of compiler-rt. I'll need to check to see what happens natively.


================
Comment at: cmake/config-ix.cmake:196
     set(${cc_out} ${COMPILER_RT_TEST_COMPILER})
     set(${cflags_out} ${COMPILER_RT_TEST_COMPILER_CFLAGS})
   else()
----------------
At this stage are COMPILER_RT_TEST_COMPILER_CFLAGS the same as COMPILER_RT_TOOLCHAIN_FLAGS?  As Line 222 cmake/base-config-ix.cmake looks to be doing

```
if(COMPILER_RT_TEST_COMPILER_CFLAGS)
    # COMPILER_RT_TEST_COMPILER_CFLAGS is used for cross-compiling.
    set(COMPILER_RT_TOOLCHAIN_CFLAGS ${COMPILER_RT_TEST_COMPILER_CFLAGS})
```
If that is the case then append to flags_out is the same on both branches.


Repository:
  rCRT Compiler Runtime

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58951/new/

https://reviews.llvm.org/D58951