[PATCH] [CMake] Cleanup tools/CMakeLists.txt to take advantage of the auto-registration that was already partially working.
Chris Bieneman
beanz at apple.com
Thu Jun 25 17:26:53 PDT 2015
> On Jun 25, 2015, at 3:30 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
> Chris Bieneman <beanz at apple.com <mailto:beanz at apple.com>> writes:
>> Hi bogner, samsonov, chapuni,
>>
>> The tools CMakeLists file already had implicit tool registration, but
>> there were a few things off about it that needed to be altered to make
>> it work. This change addresses all that. The changes in this patch
>> are:
>>
>> * factored out canonicalizing tool names from paths to CMake variables
>> * removed the LLVM_IMPLICIT_PROJECT_IGNORE mechanism in favor of
>> LLVM_EXTERNAL_${nameUPPER}_BUILD which I renamed to
>> LLVM_TOOL_${nameUPPER}_BUILD because it applies to internal and
>> external tools
>> * removed ignore_llvm_tool_subdirectory() in favor of just setting
>> LLVM_TOOL_${nameUPPER}_BUILD to Off
>> * Added create_llvm_tool_options() to resolve a bug in
>> add_llvm_external_project() - the old LLVM_EXTERNAL_${nameUPPER}_BUILD
>> would not work on a clean CMake directory because the option could be
>> created after it was set in code.
>> * Removed all but the minimum required calls to
>> add_llvm_external_project from tools/CMakeLists.txt
>
> Looks like a fairly obvious cleanup. Other than a couple of comments
> about the comments this LGTM.
>
>> http://reviews.llvm.org/D10665 <http://reviews.llvm.org/D10665>
>>
>> Files:
>> cmake/modules/AddLLVM.cmake
>> tools/CMakeLists.txt
>>
>> EMAIL PREFERENCES
>> http://reviews.llvm.org/settings/panel/emailpreferences/ <http://reviews.llvm.org/settings/panel/emailpreferences/>
>> Index: cmake/modules/AddLLVM.cmake
>> ===================================================================
>> --- cmake/modules/AddLLVM.cmake
>> +++ cmake/modules/AddLLVM.cmake
>> @@ -681,6 +681,13 @@
>> set( CURRENT_LLVM_TARGET LLVM${target_name} )
>> endmacro(add_llvm_target)
>>
>> +function(canonicalize_tool_name name output)
>> + string(REPLACE "${CMAKE_CURRENT_SOURCE_DIR}/" "" nameStrip ${name})
>> + string(REPLACE "-" "_" nameUNDERSCORE ${nameStrip})
>> + string(TOUPPER ${nameUNDERSCORE} nameUPPER)
>> + set(${output} "${nameUPPER}" PARENT_SCOPE)
>> +endfunction(canonicalize_tool_name)
>> +
>> # Add external project that may want to be built as part of llvm such as Clang,
>> # lld, and Polly. This adds two options. One for the source directory of the
>> # project, which defaults to ${CMAKE_CURRENT_SOURCE_DIR}/${name}. Another to
>> @@ -691,38 +698,43 @@
>> if("${add_llvm_external_dir}" STREQUAL "")
>> set(add_llvm_external_dir ${name})
>> endif()
>> - list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}")
>> - string(REPLACE "-" "_" nameUNDERSCORE ${name})
>> - string(TOUPPER ${nameUNDERSCORE} nameUPPER)
>> - set(LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}"
>> + canonicalize_tool_name(${name} nameUPPER)
>> + set(LLVM_TOOL_${nameUPPER}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/${add_llvm_external_dir}"
>> CACHE PATH "Path to ${name} source directory")
>> - if (NOT ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} STREQUAL ""
>> - AND EXISTS ${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)
>> - option(LLVM_EXTERNAL_${nameUPPER}_BUILD
>> + if (NOT ${LLVM_TOOL_${nameUPPER}_SOURCE_DIR} STREQUAL ""
>> + AND EXISTS ${LLVM_TOOL_${nameUPPER}_SOURCE_DIR}/CMakeLists.txt)
>> + option(LLVM_TOOL_${nameUPPER}_BUILD
>> "Whether to build ${name} as part of LLVM" ON)
>> - if (LLVM_EXTERNAL_${nameUPPER}_BUILD)
>> - add_subdirectory(${LLVM_EXTERNAL_${nameUPPER}_SOURCE_DIR} ${add_llvm_external_dir})
>> + if (LLVM_TOOL_${nameUPPER}_BUILD)
>> + add_subdirectory(${LLVM_TOOL_${nameUPPER}_SOURCE_DIR} ${add_llvm_external_dir})
>> + set(LLVM_TOOL_${nameUPPER}_BUILD Off)
>> endif()
>> endif()
>> endmacro(add_llvm_external_project)
>>
>> macro(add_llvm_tool_subdirectory name)
>> - list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE "${CMAKE_CURRENT_SOURCE_DIR}/${name}")
>> + set(LLVM_TOOL_${nameUPPER}_BUILD Off)
>> add_subdirectory(${name})
>> endmacro(add_llvm_tool_subdirectory)
>>
>> -macro(ignore_llvm_tool_subdirectory name)
>> - list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE "${CMAKE_CURRENT_SOURCE_DIR}/${name}")
>> -endmacro(ignore_llvm_tool_subdirectory)
>> +function(create_llvm_tool_options)
>> + file(GLOB sub-dirs "${CMAKE_CURRENT_SOURCE_DIR}/*")
>> + foreach(dir ${sub-dirs})
>> + if(IS_DIRECTORY "${dir}" AND EXISTS "${dir}/CMakeLists.txt")
>> + canonicalize_tool_name(${dir} name)
>> + option(LLVM_TOOL_${name}_BUILD
>> + "Whether to build ${name} as part of LLVM" On)
>> + endif()
>> + endforeach()
>> +endfunction(create_llvm_tool_options)
>>
>> -function(add_llvm_implicit_external_projects)
>> +function(add_llvm_implicit_projects)
>> set(list_of_implicit_subdirs "")
>> file(GLOB sub-dirs "${CMAKE_CURRENT_SOURCE_DIR}/*")
>> foreach(dir ${sub-dirs})
>> - if(IS_DIRECTORY "${dir}")
>> - list(FIND LLVM_IMPLICIT_PROJECT_IGNORE "${dir}" tool_subdir_ignore)
>> - if( tool_subdir_ignore EQUAL -1
>> - AND EXISTS "${dir}/CMakeLists.txt")
>> + if(IS_DIRECTORY "${dir}" AND EXISTS "${dir}/CMakeLists.txt")
>> + canonicalize_tool_name(${dir} name)
>> + if (LLVM_TOOL_${name}_BUILD)
>> get_filename_component(fn "${dir}" NAME)
>> list(APPEND list_of_implicit_subdirs "${fn}")
>> endif()
>> @@ -732,7 +744,7 @@
>> foreach(external_proj ${list_of_implicit_subdirs})
>> add_llvm_external_project("${external_proj}")
>> endforeach()
>> -endfunction(add_llvm_implicit_external_projects)
>> +endfunction(add_llvm_implicit_projects)
>>
>> # Generic support for adding a unittest.
>> function(add_unittest test_suite test_name)
>> Index: tools/CMakeLists.txt
>> ===================================================================
>> --- tools/CMakeLists.txt
>> +++ tools/CMakeLists.txt
>> @@ -1,85 +1,37 @@
>> -add_llvm_tool_subdirectory(llvm-config)
>> +# This file will recurse into all subdirectories that contain CMakeLists.txt
>> +# Setting variables that match the pattern LLVM_TOOL_{NAME}_BUILD to Off will
>> +# prevent traversing into a directory.
>> +#
>> +# The only tools that need to be explicitly added are ones that have explicit
>> +# ordering requirements.
>
> I'm not sure I understand this comment - it looks to me like we don't
> specifically add *any* tools anymore. Is this just a warning in case we
> add tools that depend on each other in the future?
This was mostly as a general warning, but it also serves as an explanation for why Polly is explicitly added below.
>
>> +
>> +# Iterates all the subdirectories to create CMake options to enable/disable
>> +# traversing each directory.
>> +create_llvm_tool_options()
>>
>> # Build polly before the tools: the tools link against polly when
>> # LINK_POLLY_INTO_TOOLS is set.
>> if(WITH_POLLY)
>> add_llvm_external_project(polly)
>> -else(WITH_POLLY)
>> - list(APPEND LLVM_IMPLICIT_PROJECT_IGNORE "${LLVM_MAIN_SRC_DIR}/tools/polly")
>> -endif(WITH_POLLY)
>> -
>> -if( LLVM_BUILD_LLVM_DYLIB )
>> - add_llvm_tool_subdirectory(llvm-shlib)
>> else()
>> - ignore_llvm_tool_subdirectory(llvm-shlib)
>> + set(LLVM_TOOL_POLLY_BUILD Off)
>> endif()
>>
>> -add_llvm_tool_subdirectory(opt)
>> -add_llvm_tool_subdirectory(llvm-as)
>> -add_llvm_tool_subdirectory(llvm-dis)
>> -add_llvm_tool_subdirectory(llvm-mc)
>> -
>> -add_llvm_tool_subdirectory(llc)
>> -add_llvm_tool_subdirectory(llvm-ar)
>> -add_llvm_tool_subdirectory(llvm-nm)
>> -add_llvm_tool_subdirectory(llvm-size)
>> -
>> -add_llvm_tool_subdirectory(llvm-cov)
>> -add_llvm_tool_subdirectory(llvm-profdata)
>> -add_llvm_tool_subdirectory(llvm-link)
>> -add_llvm_tool_subdirectory(lli)
>> -
>> -add_llvm_tool_subdirectory(llvm-extract)
>> -add_llvm_tool_subdirectory(llvm-diff)
>> -add_llvm_tool_subdirectory(macho-dump)
>> -add_llvm_tool_subdirectory(llvm-objdump)
>> -add_llvm_tool_subdirectory(llvm-readobj)
>> -add_llvm_tool_subdirectory(llvm-rtdyld)
>> -add_llvm_tool_subdirectory(llvm-dwarfdump)
>> -add_llvm_tool_subdirectory(dsymutil)
>> -add_llvm_tool_subdirectory(llvm-cxxdump)
>> -if( LLVM_USE_INTEL_JITEVENTS )
>> - add_llvm_tool_subdirectory(llvm-jitlistener)
>> -else()
>> - ignore_llvm_tool_subdirectory(llvm-jitlistener)
>> -endif( LLVM_USE_INTEL_JITEVENTS )
>> -
>> -add_llvm_tool_subdirectory(bugpoint)
>> -add_llvm_tool_subdirectory(bugpoint-passes)
>> -add_llvm_tool_subdirectory(llvm-bcanalyzer)
>> -add_llvm_tool_subdirectory(llvm-stress)
>> -add_llvm_tool_subdirectory(llvm-mcmarkup)
>> -
>> -add_llvm_tool_subdirectory(verify-uselistorder)
>> -
>> -add_llvm_tool_subdirectory(llvm-symbolizer)
>> -
>> -add_llvm_tool_subdirectory(llvm-c-test)
>> -
>> -add_llvm_tool_subdirectory(obj2yaml)
>> -add_llvm_tool_subdirectory(yaml2obj)
>> -
>> -add_llvm_tool_subdirectory(llvm-go)
>> -
>> -add_llvm_tool_subdirectory(llvm-pdbdump)
>> -
>> -if(NOT CYGWIN AND LLVM_ENABLE_PIC)
>> - add_llvm_tool_subdirectory(lto)
>> - add_llvm_tool_subdirectory(llvm-lto)
>> -else()
>> - ignore_llvm_tool_subdirectory(lto)
>> - ignore_llvm_tool_subdirectory(llvm-lto)
>> +if(NOT LLVM_BUILD_LLVM_DYLIB )
>> + set(LLVM_TOOL_LLVM_SHLIB_BUILD Off)
>> endif()
>>
>> -add_llvm_tool_subdirectory(gold)
>> +if(NOT LLVM_USE_INTEL_JITEVENTS )
>> + set(LLVM_TOOL_LLVM_JITLISTENER_BUILD Off)
>> +endif()
>>
>> -add_llvm_external_project(clang)
>> -add_llvm_external_project(llgo)
>> -add_llvm_external_project(lld)
>> -add_llvm_external_project(lldb)
>> +if(CYGWIN OR NOT LLVM_ENABLE_PIC)
>> + set(LLVM_TOOL_LTO_BUILD Off)
>> + set(LLVM_TOOL_LLVM_LTO_BUILD Off)
>> +endif()
>>
>> # Automatically add remaining sub-directories containing a 'CMakeLists.txt'
>> # file as external projects.
>
> This comment is out of date - It can probably just be "Automatically add
> all sub-directories containing a 'CMakeLists.txt'" now.
I’ll reword this. It is more accurate to say “Add everything else that wasn’t explicitly disabled”.
My vacation starts in a few hours, so I probably won’t update this until I get back.
Thanks,
-Chris
>
>> -add_llvm_implicit_external_projects()
>> +add_llvm_implicit_projects()
>>
>> set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} PARENT_SCOPE)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150625/0add4208/attachment.html>
More information about the llvm-commits
mailing list