[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