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

[Chris Bieneman via llvm-commits]

beanz added inline comments.

================
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
----------------
samsonov wrote:
> beanz wrote:
> > samsonov wrote:
> > > 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.
> > Please make up your mind over whether or not you want me to adapt the CMakeLists.txt files in this patch. In your comments here you have both asked me to make more changes to the CMakeLists files, and to make less changes.
> Sorry if I wasn't clear enough. I suggest to leave
>   foreach(arch ${XXX_SUPPORTED_ARCH})
> in every `lib/xxx/CMakeLists.txt` as is: that is, pass a single arch to `add_compiler_rt_runtime` on non-Apple platforms for now.
Why? Without this change there is no code testing the new loop.

I need you to be clear about what you want in patches. Your feedback has been very valuable, but inconsistent. Much of your feedback was calling out places where I wasn't using new functionality that these patches introduced, except this piece which is the opposite.

In writing my patches I'm trying to keep non-essential changes to a minimum, but I do want to make enough changes to exercise the new functionality. I didn't change every place to PARENT_TARGET or LINKLIBS because I figured those could be small easy follow-up patches. Your feedback has been to use those features, but not to use the multi-architecture support.

I need to understand why so that I can write my patches closer to how you want them the first time.


http://reviews.llvm.org/D12292