[PATCH] D20992: [CMake] Add LLVM runtimes directory

[Justin Bogner via llvm-commits]

Chris Bieneman <beanz at apple.com> writes:
> beanz created this revision.
> beanz added reviewers: chandlerc, bogner.
> beanz added a subscriber: llvm-commits.
>
> There are a few LLVM projects that produce runtime libraries. Ideally
> runtime libraries should be built differently than other projects,
> specifically they should be built using the just-built toolchain.
>
> There is support for building compiler-rt in this way from the clang
> build. Moving this logic into the LLVM build is interesting because it
> provides a simpler way to extend the just-built toolchain to include
> LLD and the LLVM object file tools.
>
> Once this functionality is better fleshed out and tested we’ll want to
> encapsulate it in a module that can be used for clang standalone
> builds, and we’ll want to make it the default way to build compiler-rt.
>
> With this patch applied there is no immediate change in the build.
> Moving compiler-rt out from llvm/projects into llvm/runtimes enables
> the functionality.

This seems reasonable, but I am a little worried about how transitioning
to the new system will work. Will everyone have to move their
compiler-rt checkout? Will we continue to support compiler-rt in either
place? Both of these are workable, but neither is great. Thoughts?

> This code has a few improvements over the method provided by
> LLVM_BUILD_EXTERNAL_COMPILER_RT. Specifically the sub-ninja command is
> always invoked, so changes to compiler-rt source files will get built
> properly, so this patch can be used for iterative development with
> just-built tools.
>
> http://reviews.llvm.org/D20992
>
> Files:
>   CMakeLists.txt
>   cmake/modules/LLVMExternalProjectUtils.cmake
>   runtimes/CMakeLists.txt
>
> Index: runtimes/CMakeLists.txt
> ===================================================================
> --- /dev/null
> +++ runtimes/CMakeLists.txt
> @@ -0,0 +1,16 @@
> +include(LLVMExternalProjectUtils)
> +
> +# Discover the projects that use CMake in the subdirectories.
> +# Note that explicit cmake invocation is required every time a new project is
> +# added or removed.
> +
> +add_custom_target(runtimes)
> +
> +file(GLOB entries *)
> +foreach(entry ${entries})
> +  if(IS_DIRECTORY ${entry} AND EXISTS ${entry}/CMakeLists.txt)
> +    get_filename_component(projName ${entry} NAME)
> +    llvm_ExternalProject_Add(${projName} ${entry} USE_TOOLCHAIN)
> +    add_dependencies(runtimes ${projName})
> +  endif()
> +endforeach(entry)
> Index: cmake/modules/LLVMExternalProjectUtils.cmake
> ===================================================================
> --- cmake/modules/LLVMExternalProjectUtils.cmake
> +++ cmake/modules/LLVMExternalProjectUtils.cmake
> @@ -29,7 +29,8 @@
>  #     Extra targets in the subproject to generate targets for
>  #   )
>  function(llvm_ExternalProject_Add name source_dir)
> -  cmake_parse_arguments(ARG "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL"
> +  cmake_parse_arguments(ARG
> +    "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL;ALWAYS_CLEAN"
>      "SOURCE_DIR"
>      "CMAKE_ARGS;TOOLCHAIN_TOOLS;RUNTIME_LIBRARIES;DEPENDS;EXTRA_TARGETS" ${ARGN})
>    canonicalize_tool_name(${name} nameCanon)
> @@ -52,6 +53,10 @@
>      endif()
>    endforeach()
>  
> +  if(ARG_ALWAYS_CLEAN)
> +    set(always_clean clean)
> +  endif()
> +
>    list(FIND TOOLCHAIN_TOOLS clang FOUND_CLANG)
>    if(FOUND_CLANG GREATER -1)
>      set(CLANG_IN_TOOLCHAIN On)
> @@ -135,6 +140,14 @@
>      CMAKE_ARGS ${${nameCanon}_CMAKE_ARGS}
>                 ${compiler_args}
>                 -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
> +               -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config
> +               -DLLVM_LIBRARY_OUTPUT_INTDIR=${LLVM_LIBRARY_OUTPUT_INTDIR}
> +               -DLLVM_RUNTIME_OUTPUT_INTDIR=${LLVM_RUNTIME_OUTPUT_INTDIR}
> +               -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}
> +               -DLLVM_ENABLE_WERROR=${LLVM_ENABLE_WERROR}
> +               -DPACKAGE_VERSION=${PACKAGE_VERSION}
> +               -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
> +               -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}

How did you decide which variables need to be passed through like this?
The set seems somewhat arbitrary, but I may be missing something
obvious.

>                 ${ARG_CMAKE_ARGS}
>                 ${PASSTHROUGH_VARIABLES}
>      INSTALL_COMMAND ""
> @@ -152,7 +165,7 @@
>      ExternalProject_Add_Step(${name} force-rebuild
>        COMMAND ${run_build}
>        COMMENT "Forcing rebuild of ${name}"
> -      DEPENDEES configure clean
> +      DEPENDEES configure ${always_clean}

I'm not sure I understand what this does. If I had to guess I'd say that
when ALWAYS_CLEAN is passed the rebuild of the external project always
invokes clean first, but if it's not passed we'll just invoke the
external build and allow it to be incremental if appropriate. Is that
right?

>        DEPENDS ${ALWAYS_REBUILD} ${ARG_DEPENDS} ${TOOLCHAIN_BINS}
>        ${cmake_3_4_USES_TERMINAL} )
>    endif()
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt
> +++ CMakeLists.txt
> @@ -720,6 +720,8 @@
>    add_subdirectory(tools)
>  endif()
>  
> +add_subdirectory(runtimes)
> +
>  if( LLVM_INCLUDE_EXAMPLES )
>    add_subdirectory(examples)
>  endif()
> @@ -730,7 +732,8 @@
>      llvm_ExternalProject_Add(test-suite ${LLVM_MAIN_SRC_DIR}/projects/test-suite
>        USE_TOOLCHAIN
>        EXCLUDE_FROM_ALL
> -      NO_INSTALL)
> +      NO_INSTALL
> +      ALWAYS_CLEAN)
>    endif()
>    add_subdirectory(test)
>    add_subdirectory(unittests)
>