[PATCH] D12292: [CMake] merge add_compiler_rt_runtime and add_compiler_rt_darwin_runtime into a single function

[Alexey Samsonov via llvm-commits]

samsonov added inline comments.

================
Comment at: cmake/Modules/AddCompilerRT.cmake:60
@@ +59,3 @@
+#                         LINK_LIBS <linked libraries> (only for shared library)
+#                         OUTPUT_NAME <output library name>
+#                         PARENT_TARGET <convenience parent target>)
----------------
There is no such argument now.

================
Comment at: cmake/Modules/AddCompilerRT.cmake:79
@@ +78,3 @@
+        set(libname "${name}_${os}_dynamic")
+        set(extra_linkflags_${libname} ${DARWIN_${os}_LINKFLAGS} ${LIB_CFLAGS} ${LIB_LINKFLAGS})
+      endif()
----------------
you don't use LIB_CFLAGS in add_compiler_rt_darwin_runtime(.. SHARED ...).

================
Comment at: cmake/Modules/AddCompilerRT.cmake:99
@@ +98,3 @@
+        if(WIN32)
+          set(output_name_${libname} ${name}_dynamic-${arch}${COMPILER_RT_OS_SUFFIX})
+        endif()
----------------
You should also set output_name for shared libraries on Unix - ${name}-${arch}${COMPILER_RT_OS_SUFFIX} (note: no `dynamic` here)... which probably means it's easier to set output_name_${libname} for *all* libraries here, in this loop, and then just remove special case from the loop below.

================
Comment at: lib/asan/CMakeLists.txt:194
@@ -190,3 +193,3 @@
+      PARENT_TARGET asan)
     target_link_libraries(clang_rt.asan-dynamic-${arch} ${ASAN_DYNAMIC_LIBS})
 
----------------
Pass ${ASAN_DYNAMIC_LIBS} to `add_compiler_rt_runtime` as `LINK_LIBS`?

================
Comment at: lib/profile/CMakeLists.txt:21
@@ -21,7 +20,3 @@
 else()
-  foreach(arch ${PROFILE_SUPPORTED_ARCH})
-    add_compiler_rt_runtime(clang_rt.profile-${arch} ${arch} STATIC
-      CFLAGS -fPIC
-      SOURCES ${PROFILE_SOURCES})
-    add_dependencies(profile clang_rt.profile-${arch})
-  endforeach()
+  add_compiler_rt_runtime(clang_rt.profile
+    STATIC
----------------
Please leave the loop over PROFILE_SUPPORTED_ARCH here for this change, and let's collapse it here (and in similar places, like lsan) in subsequent changes.

================
Comment at: lib/tsan/CMakeLists.txt:112
@@ -109,2 +111,3 @@
             $<TARGET_OBJECTS:RTUbsan.${arch}>
     CFLAGS ${TSAN_RTL_CFLAGS})
+  add_compiler_rt_runtime(clang_rt.tsan_cxx
----------------
PARENT_TARGET tsan

================
Comment at: lib/tsan/dd/CMakeLists.txt:43
@@ +42,3 @@
+            $<TARGET_OBJECTS:RTSanitizerCommonLibc.${arch}>
+    LIBS ${DD_LINKLIBS}
+    PARENT_TARGET dd)
----------------
Now this is named `LINK_LIBS`


http://reviews.llvm.org/D12292