[PATCH] D31098: [compiler-rt] respect CMAKE_EXE_LINKER_FLAGS in compiler and library tests

[Chris Bieneman via Phabricator via llvm-commits]

beanz added a comment.

@davidxl, do you recall how the failure that caused you to make the policy change exhibited itself? It seems to me like we were working around the behavior of the old policy setting by passing the linker options through manually. I wonder if the reason for the error was CMake masking those liker flags under the new policy.

Thoughts?



================
Comment at: CMakeLists.txt:17
 cmake_minimum_required(VERSION 3.4.3)
-# FIXME:
-# The OLD behavior (pre 3.2) for this policy is to not set the value of the 
-# CMAKE_EXE_LINKER_FLAGS variable in the generated test project. The NEW behavior
-# for this policy is to set the value of the CMAKE_EXE_LINKER_FLAGS variable 
-# in the test project to the same as it is in the calling project. The new 
-# behavior cause the compiler_rt test to fail during try_compile: see
-# projects/compiler-rt/cmake/Modules/CompilerRTUtils.cmake:121 such that
-# CAN_TARGET_${arch} is not set properly. This results in COMPILER_RT_SUPPORTED_ARCH
-# not being updated properly leading to poblems.
-cmake_policy(SET CMP0056 OLD)
+cmake_policy(SET CMP0056 NEW)
 
----------------
You shouldn't need to set this to NEW, it should default to NEW based on the `cmake_minimum_version` being set above.


================
Comment at: cmake/Modules/CompilerRTUtils.cmake:129
+  set(SAVED_CMAKE_EXE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS})
+  set(CMAKE_EXE_LINKER_FLAGS "")
   set(TARGET_${arch}_CFLAGS ${ARGN})
----------------
Pretty sure this will actually change behavior. Look down below, we were reading this value and passing it through to the `try_compile` call.


https://reviews.llvm.org/D31098