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

Thu Jun 9 09:38:31 PDT 2016

In this email I’m going to focus on patch feedback. I’ll send a separate one to talk about the higher-level discussions.

> On Jun 8, 2016, at 4:38 PM, Justin Bogner <mail at justinbogner.com> wrote:
> 
> Chris Bieneman <beanz at apple.com <mailto: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.

I’m trying to define a standardized interface for runtime projects. If you look in the clang/runtimes/CMakeLists.txt file there is crazy handling to make this work for compiler-rt. This is my stab at trying to standardize that so that it could be applied to all runtime projects.

Making this work for compiler-rt did require a small patch to compiler-rt’s build system (r271749). I expect similar patches will be needed for other runtime projects too. This will need to be taken into account in any migration plan.

If you want I can go through each one and explain why I included them, but the general idea is that the CMAKE_* options are needed to pass through expected settings or deal with non-standard system configurations (like your build tool not being in PATH). The LLVM_* variables are my attempt at defining a standard interface for runtime projects so that their build behavior will seem as if they are in-tree configured projects. This makes it so the clang in your build tree will properly find the runtime libraries.

> 
>>                ${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?

The ALWAYS_CLEAN functionality is used by the test-suite. It is so that the test-suite target always rebuilds everything. Your guess is correct for how it works.

-Chris

> 
>>       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)

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