[PATCH] [CMake] Cleanup add_compiler_rt_object_library to be platform-agnostic

Richard Smith richard at metafoo.co.uk
Thu Jun 4 13:47:49 PDT 2015


================
Comment at: cmake/Modules/AddCompilerRT.cmake:6-7
@@ -5,3 +5,4 @@
 
 # Tries to add "object library" target for a given architecture
 # with name "<name>.<arch>" if architecture can be targeted.
+# add_compiler_rt_object_library(<name>
----------------
This comment needs to be updated.

================
Comment at: lib/asan/CMakeLists.txt:79
@@ +78,3 @@
+  add_compiler_rt_object_library(RTAsan
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
+    ARCH ${ASAN_SUPPORTED_ARCH}
----------------
It seems weird for the dynamic library target to have the name `RTAsan` for `APPLE` but to be named `RTAsan_dynamic` everywhere else. Is it possible to merge these two targets and move them out of the `if` here? (Not as part of this change, just curious if we can unify this further.)

================
Comment at: lib/interception/CMakeLists.txt:15-27
@@ -14,16 +14,14 @@
 
 if(APPLE)
   # Build universal binary on APPLE.
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTInterception ${os}
-      ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
-      SOURCES ${INTERCEPTION_SOURCES}
-      CFLAGS ${INTERCEPTION_CFLAGS})
-  endforeach()
+  add_compiler_rt_object_library(RTInterception
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
+    ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
+    SOURCES ${INTERCEPTION_SOURCES}
+    CFLAGS ${INTERCEPTION_CFLAGS})
 else()
   # Otherwise, build separate libraries for each target.
-  foreach(arch ${SANITIZER_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTInterception ${arch}
-      SOURCES ${INTERCEPTION_SOURCES} CFLAGS ${INTERCEPTION_CFLAGS})
-  endforeach()
+  add_compiler_rt_object_library(RTInterception
+    ARCH ${SANITIZER_COMMON_SUPPORTED_ARCH}
+    SOURCES ${INTERCEPTION_SOURCES} CFLAGS ${INTERCEPTION_CFLAGS})
 endif()
----------------
Do you still need the `if` here? Both arms look the same (other than the `OS` part which is ignored for the non-`APPLE` case).

================
Comment at: lib/lsan/CMakeLists.txt:21-31
@@ -20,15 +20,13 @@
 
 if(APPLE)
-  foreach(os ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS})
-    add_compiler_rt_darwin_object_library(RTLSanCommon ${os}
-      ARCH ${LSAN_COMMON_SUPPORTED_ARCH}
-      SOURCES ${LSAN_COMMON_SOURCES}
-      CFLAGS ${LSAN_CFLAGS})
-  endforeach()
+  add_compiler_rt_object_library(RTLSanCommon
+    OS ${SANITIZER_COMMON_SUPPORTED_DARWIN_OS}
+    ARCH ${LSAN_COMMON_SUPPORTED_ARCH}
+    SOURCES ${LSAN_COMMON_SOURCES}
+    CFLAGS ${LSAN_CFLAGS})
 else()
-  foreach(arch ${LSAN_COMMON_SUPPORTED_ARCH})
-    add_compiler_rt_object_library(RTLSanCommon ${arch}
-      SOURCES ${LSAN_COMMON_SOURCES}
-      CFLAGS ${LSAN_CFLAGS})
-  endforeach()
+  add_compiler_rt_object_library(RTLSanCommon
+    ARCH ${LSAN_COMMON_SUPPORTED_ARCH}
+    SOURCES ${LSAN_COMMON_SOURCES}
+    CFLAGS ${LSAN_CFLAGS})
 
----------------
Likewise, can you move this part out of the `if`?

http://reviews.llvm.org/D10250

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list