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

[Chris Bieneman via llvm-commits]

> On Aug 10, 2015, at 1:50 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> 
> Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> writes:
>> beanz added a comment.
>> 
>> I'll send updated patches shortly. Comments inline below.
>> 
>> 
>> ================
>> Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:31
>> @@ +30,3 @@
>> +
>> +  string(REGEX MATCH "configured to support archs: (.*)\nLTO support.*"
>> +         ARCHES_MATCHED "${LINKER_VERSION}")
>> ----------------
>> filcab wrote:
>>> Nit: Do we care about having the "LTO support" string?
>> The "LTO Support" string is needed to escape the end of the
>> regex. Since .* matches everything and nothing, and the behavior is to
>> match as much as possible without the "LTO Support" string it matches
>> the whole rest of the output.
>> 
>> I'm no regex expert though, so if you have a better suggestion I'm open.
> 
> Presumably "configured to support archs: ([^\n]*)\n" would do what you want.

This is almost exactly what Fil suggested in IRC, and I updated the patches already :-)

-Chris

> 
>> 
>> ================
>> Comment at: cmake/Modules/CompilerRTDarwinUtils.cmake:63
>> @@ +62,3 @@
>> +    list(REMOVE_ITEM archs "x86_64h")
>> +  endif()
>> +
>> ----------------
>> filcab wrote:
>>> "will build", but then you remove that arch?
>>> Why is the `iossim` different from OS X, here?
>> I should explain better. The trivial test program does compile for
>> x86_64h because it falls back to using the x86_64 libraries, but
>> x86_64h isn't actually a valid simulator architecture, so other things
>> will fail. I've had trouble finding a non-hacky way to make that
>> detection work.
>> 
>> ================
>> Comment at: cmake/config-ix.cmake:318
>> @@ +317,3 @@
>> +  
>> +  if(NOT MACOSX_VERSION_MIN_FLAG)
>> +    darwin_test_archs(osx
>> ----------------
>> filcab wrote:
>>> 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)
>> The behavior if the user set macos-version-min explicitly is kinda
>> complicated and probably broken. As it is now, I'm pretty sure we just
>> don't build the sanitizers if you specify an explicit version min.
>> 
>> The rationale for skipping iOS and the simulator I assume is because
>> we don't want to change the user's CXX flags, and there may be other
>> incompatibilities. I don't know for sure though because that code
>> exists in-tree today. This patch just moves it from the root
>> CMakeLists.txt to config-ix as per samsonov's comments on previous
>> reviews.
>> 
>> ================
>> 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
>> ----------------
>> filcab wrote:
>>> 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?
>> You're right. Since the driver should be passing this through we don't
>> need it explicitly set.
>> 
>> 
>> http://reviews.llvm.org/D11820

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150810/b3714657/attachment.html>