[PATCH] D15133: [CMake] Add option LLVM_EXTERNALIZE_DEBUGINFO

[Chris Bieneman via llvm-commits]

> On Dec 2, 2015, at 2:02 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> writes:
>> beanz updated this revision to Diff 41660.
>> beanz added a comment.
>> 
>> For LTO builds, dsymutil doesn't need to run against the intermediate
>> object, we just need to make sure it sticks around.
>> 
>> 
>> http://reviews.llvm.org/D15133
>> 
>> Files:
>>  CMakeLists.txt
>>  cmake/modules/AddLLVM.cmake
>> 
>> Index: cmake/modules/AddLLVM.cmake
>> ===================================================================
>> --- cmake/modules/AddLLVM.cmake
>> +++ cmake/modules/AddLLVM.cmake
>> @@ -512,6 +512,10 @@
>>       add_dependencies(${objlib} ${LLVM_COMMON_DEPENDS})
>>     endforeach()
>>   endif()
>> +
>> +  if(ARG_SHARED OR ARG_MODULE)
>> +    llvm_externalize_darwin_debuginfo(${name})
>> +  endif()
>> endfunction()
>> 
>> macro(add_llvm_library name)
>> @@ -655,6 +659,8 @@
>>   if( LLVM_COMMON_DEPENDS )
>>     add_dependencies( ${name} ${LLVM_COMMON_DEPENDS} )
>>   endif( LLVM_COMMON_DEPENDS )
>> +
>> +  llvm_externalize_darwin_debuginfo(${name})
>> endmacro(add_llvm_executable name)
>> 
>> function(export_executable_symbols target)
>> @@ -1168,3 +1174,18 @@
>>     endif()
>>   endif()
>> endfunction()
>> +
>> +function(llvm_externalize_darwin_debuginfo name)
>> +  if(APPLE AND LLVM_EXTERNALIZE_DEBUGINFO)
> 
> Better to early exit `if (NOT LLVM_EXTERNALIZE_DEBUGINFO)` before
> checking APPLE.
> 
> I'd also prefer if we error'd out on non-darwin for now. I don't like
> silently ignored flags, and if someone wants to implement it later this
> is the obvious place to do split-dwarf on linux-like platforms. Then
> presumably the "_darwin" isn't necessary in the function name.

will do.

> 
>> +    if(CMAKE_CXX_FLAGS MATCHES "-flto"
>> +      OR CMAKE_CXX_FLAGS_${uppercase_CMAKE_BUILD_TYPE} MATCHES "-flto")
>> +
>> +      set(lto_object ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR}/${name}.o)
> 
> Can this collide with anything? Maybe ${name}.lto.o is safer?

It shouldn’t because CMake doesn’t put objects in that folder, but I updated the patches with a similar change as per Fred’s comments :-).

-Chris

> 
> Other than those couple of points, this LGTM.
> 
>> +      set_target_properties(${name} PROPERTIES
>> +        LINK_FLAGS "-Wl,-object_path_lto -Wl,${lto_object}")
>> +    endif()
>> +    add_custom_command(TARGET ${name} POST_BUILD
>> +      COMMAND xcrun dsymutil $<TARGET_FILE:${name}>
>> +      COMMAND xcrun strip -Sl $<TARGET_FILE:${name}>)
>> +  endif()
>> +endfunction()
>> Index: CMakeLists.txt
>> ===================================================================
>> --- CMakeLists.txt
>> +++ CMakeLists.txt
>> @@ -301,6 +301,9 @@
>> option(LLVM_USE_OPROFILE
>>   "Use opagent JIT interface to inform OProfile about JIT code" OFF)
>> 
>> +option(LLVM_EXTERNALIZE_DEBUGINFO
>> +  "Generate dSYM files and strip executables and libraries (Darwin Only)" OFF)
>> +
>> # If enabled, verify we are on a platform that supports oprofile.
>> if( LLVM_USE_OPROFILE )
>>   if( NOT CMAKE_SYSTEM_NAME MATCHES "Linux" )

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