[PATCH] [CMake] Cleanup tools/CMakeLists.txt to take advantage of the auto-registration that was already partially working.
Justin Bogner
mail at justinbogner.com
Thu Jun 25 15:30:57 PDT 2015
Chris Bieneman <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
>
> Files:
> cmake/modules/AddLLVM.cmake
> tools/CMakeLists.txt
>
> EMAIL PREFERENCES
> 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?
> +
> +# 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.
> -add_llvm_implicit_external_projects()
> +add_llvm_implicit_projects()
>
> set(LLVM_COMMON_DEPENDS ${LLVM_COMMON_DEPENDS} PARENT_SCOPE)
More information about the llvm-commits
mailing list