[PATCH] D11820: [CMake] Add experimental support for building compiler-rt for iOS

[Filipe Cabecinhas via llvm-commits]

filcab added a subscriber: filcab.
filcab added a comment.

Minor nits and comments inline.
Thanks a lot for working on this!


================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:31
@@ +30,3 @@
+
+  string(REGEX MATCH "configured to support archs: (.*)\nLTO support.*"
+         ARCHES_MATCHED "${LINKER_VERSION}")
----------------
Nit: Do we care about having the "LTO support" string?

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:63
@@ +62,3 @@
+    list(REMOVE_ITEM archs "x86_64h")
+  endif()
+
----------------
"will build", but then you remove that arch?
Why is the `iossim` different from OS X, here?

================
Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:65
@@ +64,3 @@
+
+  set(WORKING_ARCHES)
+  foreach(arch ${archs})
----------------
Nit: You've consistently kept local variable all_lowercase except for this one.

================
Comment at: cmake/config-ix.cmake:318
@@ +317,3 @@
+  
+  if(NOT MACOSX_VERSION_MIN_FLAG)
+    darwin_test_archs(osx
----------------
What's the behavior you want if `MACOSX_VERSION_MIN_FLAG` is set? I'm guessing you're going with "just assume the user just wants OS X, since they passed that flag in the CXX flags".
Shouldn't we at least test the OS X archs?

Why are we skipping iOS and iOS Simulator when we have a specific minimum OS X version? (I'm good with "so we don't change the user's CXX flags", but want to double-check)

================
Comment at: cmake/config-ix.cmake:335
@@ +334,3 @@
+        ${DARWIN_COMMON_LINKFLAGS}
+        -Wl,-ios_simulator_version_min,7.0.0
+        -mios-simulator-version-min=7.0
----------------
Does clang not pass the proper flag to the linker for the iOS Simulator (when the flag below is passed)?
If not, are they both needed, still? Since clang should only be invoking the linker.
Why didn't we need this for OS X? Why are the versions slightly different?

================
Comment at: cmake/config-ix.cmake:357
@@ +356,3 @@
+        ${DARWIN_COMMON_LINKFLAGS}
+        -Wl,-ios_version_min,7.0.0
+        -miphoneos-version-min=7.0
----------------
Same as for iOS Simulator.


http://reviews.llvm.org/D11820