<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, Jun 8, 2016 at 4:39 PM Justin Bogner via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Chris Bieneman <<a href="mailto:beanz@apple.com" target="_blank">beanz@apple.com</a>> writes:<br>
> beanz created this revision.<br>
> beanz added reviewers: chandlerc, bogner.<br>
> beanz added a subscriber: llvm-commits.<br>
><br>
> There are a few LLVM projects that produce runtime libraries. Ideally<br>
> runtime libraries should be built differently than other projects,<br>
> specifically they should be built using the just-built toolchain.<br>
><br>
> There is support for building compiler-rt in this way from the clang<br>
> build. Moving this logic into the LLVM build is interesting because it<br>
> provides a simpler way to extend the just-built toolchain to include<br>
> LLD and the LLVM object file tools.<br>
><br>
> Once this functionality is better fleshed out and tested we’ll want to<br>
> encapsulate it in a module that can be used for clang standalone<br>
> builds, and we’ll want to make it the default way to build compiler-rt.<br>
><br>
> With this patch applied there is no immediate change in the build.<br>
> Moving compiler-rt out from llvm/projects into llvm/runtimes enables<br>
> the functionality.<br>
<br>
This seems reasonable, but I am a little worried about how transitioning<br>
to the new system will work. Will everyone have to move their<br>
compiler-rt checkout? Will we continue to support compiler-rt in either<br>
place? Both of these are workable, but neither is great. Thoughts?<br></blockquote><div><br></div><div>I share your concerns, but I also kind of like the direction this is going.</div><div><br></div><div>But there is a higher-level meta-point: do we want to keep the 'projects' directory *at all*.</div><div><br></div><div>Every single resident of it I can think of except for the test-suite is either dead (dragonegg) or a runtime library.</div><div><br></div><div>I think we should either have all the build-integrated projects in a single 'projects' directory (including LLD and Clang), or we should have none of them and use more domain relevant organization (today "tools", you're adding "runtimes", maybe we move the test-suite to go under one of the test directories).</div><div><br></div><div>I think we should have a consistent plan here before moving stuff. But once we have it, I think we shouldn't be afraid of re-organizing stuff to make more sense, and just work to get folks to update their checkouts.</div><div><br></div><div>-Chandler</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> This code has a few improvements over the method provided by<br>
> LLVM_BUILD_EXTERNAL_COMPILER_RT. Specifically the sub-ninja command is<br>
> always invoked, so changes to compiler-rt source files will get built<br>
> properly, so this patch can be used for iterative development with<br>
> just-built tools.<br>
><br>
> <a href="http://reviews.llvm.org/D20992" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20992</a><br>
><br>
> Files:<br>
> CMakeLists.txt<br>
> cmake/modules/LLVMExternalProjectUtils.cmake<br>
> runtimes/CMakeLists.txt<br>
><br>
> Index: runtimes/CMakeLists.txt<br>
> ===================================================================<br>
> --- /dev/null<br>
> +++ runtimes/CMakeLists.txt<br>
> @@ -0,0 +1,16 @@<br>
> +include(LLVMExternalProjectUtils)<br>
> +<br>
> +# Discover the projects that use CMake in the subdirectories.<br>
> +# Note that explicit cmake invocation is required every time a new project is<br>
> +# added or removed.<br>
> +<br>
> +add_custom_target(runtimes)<br>
> +<br>
> +file(GLOB entries *)<br>
> +foreach(entry ${entries})<br>
> + if(IS_DIRECTORY ${entry} AND EXISTS ${entry}/CMakeLists.txt)<br>
> + get_filename_component(projName ${entry} NAME)<br>
> + llvm_ExternalProject_Add(${projName} ${entry} USE_TOOLCHAIN)<br>
> + add_dependencies(runtimes ${projName})<br>
> + endif()<br>
> +endforeach(entry)<br>
> Index: cmake/modules/LLVMExternalProjectUtils.cmake<br>
> ===================================================================<br>
> --- cmake/modules/LLVMExternalProjectUtils.cmake<br>
> +++ cmake/modules/LLVMExternalProjectUtils.cmake<br>
> @@ -29,7 +29,8 @@<br>
> # Extra targets in the subproject to generate targets for<br>
> # )<br>
> function(llvm_ExternalProject_Add name source_dir)<br>
> - cmake_parse_arguments(ARG "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL"<br>
> + cmake_parse_arguments(ARG<br>
> + "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL;ALWAYS_CLEAN"<br>
> "SOURCE_DIR"<br>
> "CMAKE_ARGS;TOOLCHAIN_TOOLS;RUNTIME_LIBRARIES;DEPENDS;EXTRA_TARGETS" ${ARGN})<br>
> canonicalize_tool_name(${name} nameCanon)<br>
> @@ -52,6 +53,10 @@<br>
> endif()<br>
> endforeach()<br>
><br>
> + if(ARG_ALWAYS_CLEAN)<br>
> + set(always_clean clean)<br>
> + endif()<br>
> +<br>
> list(FIND TOOLCHAIN_TOOLS clang FOUND_CLANG)<br>
> if(FOUND_CLANG GREATER -1)<br>
> set(CLANG_IN_TOOLCHAIN On)<br>
> @@ -135,6 +140,14 @@<br>
> CMAKE_ARGS ${${nameCanon}_CMAKE_ARGS}<br>
> ${compiler_args}<br>
> -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}<br>
> + -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config<br>
> + -DLLVM_LIBRARY_OUTPUT_INTDIR=${LLVM_LIBRARY_OUTPUT_INTDIR}<br>
> + -DLLVM_RUNTIME_OUTPUT_INTDIR=${LLVM_RUNTIME_OUTPUT_INTDIR}<br>
> + -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}<br>
> + -DLLVM_ENABLE_WERROR=${LLVM_ENABLE_WERROR}<br>
> + -DPACKAGE_VERSION=${PACKAGE_VERSION}<br>
> + -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}<br>
> + -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}<br>
<br>
How did you decide which variables need to be passed through like this?<br>
The set seems somewhat arbitrary, but I may be missing something<br>
obvious.<br>
<br>
> ${ARG_CMAKE_ARGS}<br>
> ${PASSTHROUGH_VARIABLES}<br>
> INSTALL_COMMAND ""<br>
> @@ -152,7 +165,7 @@<br>
> ExternalProject_Add_Step(${name} force-rebuild<br>
> COMMAND ${run_build}<br>
> COMMENT "Forcing rebuild of ${name}"<br>
> - DEPENDEES configure clean<br>
> + DEPENDEES configure ${always_clean}<br>
<br>
I'm not sure I understand what this does. If I had to guess I'd say that<br>
when ALWAYS_CLEAN is passed the rebuild of the external project always<br>
invokes clean first, but if it's not passed we'll just invoke the<br>
external build and allow it to be incremental if appropriate. Is that<br>
right?<br>
<br>
> DEPENDS ${ALWAYS_REBUILD} ${ARG_DEPENDS} ${TOOLCHAIN_BINS}<br>
> ${cmake_3_4_USES_TERMINAL} )<br>
> endif()<br>
> Index: CMakeLists.txt<br>
> ===================================================================<br>
> --- CMakeLists.txt<br>
> +++ CMakeLists.txt<br>
> @@ -720,6 +720,8 @@<br>
> add_subdirectory(tools)<br>
> endif()<br>
><br>
> +add_subdirectory(runtimes)<br>
> +<br>
> if( LLVM_INCLUDE_EXAMPLES )<br>
> add_subdirectory(examples)<br>
> endif()<br>
> @@ -730,7 +732,8 @@<br>
> llvm_ExternalProject_Add(test-suite ${LLVM_MAIN_SRC_DIR}/projects/test-suite<br>
> USE_TOOLCHAIN<br>
> EXCLUDE_FROM_ALL<br>
> - NO_INSTALL)<br>
> + NO_INSTALL<br>
> + ALWAYS_CLEAN)<br>
> endif()<br>
> add_subdirectory(test)<br>
> add_subdirectory(unittests)<br>
><br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div>