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

Chris Bieneman beanz at apple.com
Thu Jun 4 14:06:06 PDT 2015


I will send updated patches in a minute once I finish testing them.


================
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>
----------------
rsmith wrote:
> This comment needs to be updated.
Will do.

================
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}
----------------
rsmith wrote:
> 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.)
I hope to reduce this further in another patch. I really want to get to the point where there is no platform handling in the CMakeLists files for libraries.

================
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()
----------------
rsmith wrote:
> 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).
Yea, I can get rid of the if here. I was going to do that in a later patch, because I didn't realize the OS parameter was the only difference.

================
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})
 
----------------
rsmith wrote:
> Likewise, can you move this part out of the `if`?
Yep, I can do that here too. That will get rid of the Apple side of this conditional.

http://reviews.llvm.org/D10250

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






More information about the llvm-commits mailing list