<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Moving to llvm-dev (I think this has gone a bit further than a patch review discussion)</div><div class=""><br class=""></div><div class="">In hindsight I probably should have explained more of my thinking on this with the patch, or done an RFC on llvm-dev to start with. I’l do that now, and answer the questions along the way. I sent a separate email discussing Justin’s patch review feedback.</div><div class=""><br class=""></div><div class="">In the build system today there is no strong distinction between ‘projects’ and ‘tools’. There are a few subtle differences, but I’m not sure any of them really matter. The differences are:</div><div class=""><br class=""></div><div class="">(1) The projects directory is always configured, tools can be disabled using LLVM_INCLUDE_TOOLS=Off (projects and tools can both be individually disabled too)</div><div class="">(2) Projects are configured before tools, so tools can rely on targets being created for projects (we don’t really use this, and anywhere we are is probably a bug)</div><div class="">(3) Some projects have special handling. For example test-suite isn’t actually treated as a project, it has special handling in LLVM/CMakeLists.txt:727, and Compiler-RT is handled by clang if you set LLVM_BUILD_EXTERNAL_COMPILER_RT=On.</div><div class=""><br class=""></div><div class="">With this in mind I was thinking about the general usability of our build system. The distinction between a project and a tool is not very clear. At a high level I see three different use cases that are covered by our current projects & tools directories.</div><div class=""><br class=""></div><div class="">(1) Projects that are configured with LLVM</div><div class="">(2) Runtime projects that should be configured using the just-built tools</div><div class="">(3) The LLVM test-suite, which is really just external tests that should be configured and run with the just-built tools</div><div class=""><br class=""></div><div class="">My proposal is that we make the tools subdirectory the *only* place for projects that fall into category 1. I don’t think there is any technical reason to drop an in-tree project into projects over tools today, and I think we migrating people who are doing that away from it should be easy.</div><div class=""><br class=""></div><div class="">Second I want to add a “runtimes” directory to LLVM to cover case 2 (see D20992). The idea behind this is to use common code in LLVM to support building runtimes. This will allow the full LLVM toolchain to be visible during configuration. I will abstract this functionality into an installed CMake module so that Clang can use it for out-of-tree clang builds.</div><div class=""><br class=""></div><div class="">Lastly we need to give the test-suite a new home. I’m not super concerned with where we do that. It could be under tests, it could just be at the root of the LLVM directory. I don’t think it matters too much because it is a one-off. Thoughts welcome.</div><div class=""><br class=""></div><div class="">My proposed patch makes the runtimes directory work for Compiler-RT, but it doesn’t yet handle libcxxabi, libcxx and libunwind. There is some special case handling between libcxxabi and libcxx that will need to be handled to make the dependencies work between the two, and I still need to work that out.</div><div class=""><br class=""></div><div class="">If we want to go with this proposal I envision the transition being multi-staged:</div><div class=""><br class=""></div><div class="">(1) Adding the new functionality, getting it up and fully working for all runtime projects - this will involve changes to runtime projects</div><div class="">(2) Work with bot maintainers to migrate bots, and fix any issues that come up</div><div class="">(3) Add support for a new secondary location for the test-suite</div><div class="">(4) Set a date for removing the projects directory, post patches including updated documentation</div><div class="">(5) Remove the projects directory entirely</div><div class=""><br class=""></div><div class="">Thoughts?</div><div class="">-Chris</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 8, 2016, at 6:59 PM, Chandler Carruth <<a href="mailto:chandlerc@gmail.com" class="">chandlerc@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_quote"><div dir="ltr" class="">On Wed, Jun 8, 2016 at 4:39 PM Justin Bogner via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;">Chris Bieneman <<a href="mailto:beanz@apple.com" target="_blank" class="">beanz@apple.com</a>> writes:<br class="">> beanz created this revision.<br class="">> beanz added reviewers: chandlerc, bogner.<br class="">> beanz added a subscriber: llvm-commits.<br class="">><br class="">> There are a few LLVM projects that produce runtime libraries. Ideally<br class="">> runtime libraries should be built differently than other projects,<br class="">> specifically they should be built using the just-built toolchain.<br class="">><br class="">> There is support for building compiler-rt in this way from the clang<br class="">> build. Moving this logic into the LLVM build is interesting because it<br class="">> provides a simpler way to extend the just-built toolchain to include<br class="">> LLD and the LLVM object file tools.<br class="">><br class="">> Once this functionality is better fleshed out and tested we’ll want to<br class="">> encapsulate it in a module that can be used for clang standalone<br class="">> builds, and we’ll want to make it the default way to build compiler-rt.<br class="">><br class="">> With this patch applied there is no immediate change in the build.<br class="">> Moving compiler-rt out from llvm/projects into llvm/runtimes enables<br class="">> the functionality.<br class=""><br class="">This seems reasonable, but I am a little worried about how transitioning<br class="">to the new system will work. Will everyone have to move their<br class="">compiler-rt checkout? Will we continue to support compiler-rt in either<br class="">place? Both of these are workable, but neither is great. Thoughts?<br class=""></blockquote><div class=""><br class=""></div><div class="">I share your concerns, but I also kind of like the direction this is going.</div><div class=""><br class=""></div><div class="">But there is a higher-level meta-point: do we want to keep the 'projects' directory *at all*.</div><div class=""><br class=""></div><div class="">Every single resident of it I can think of except for the test-suite is either dead (dragonegg) or a runtime library.</div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">-Chandler</div><div class=""> </div><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><br class="">> This code has a few improvements over the method provided by<br class="">> LLVM_BUILD_EXTERNAL_COMPILER_RT. Specifically the sub-ninja command is<br class="">> always invoked, so changes to compiler-rt source files will get built<br class="">> properly, so this patch can be used for iterative development with<br class="">> just-built tools.<br class="">><br class="">><span class="Apple-converted-space"> </span><a href="http://reviews.llvm.org/D20992" rel="noreferrer" target="_blank" class="">http://reviews.llvm.org/D20992</a><br class="">><br class="">> Files:<br class="">> CMakeLists.txt<br class="">> cmake/modules/LLVMExternalProjectUtils.cmake<br class="">> runtimes/CMakeLists.txt<br class="">><br class="">> Index: runtimes/CMakeLists.txt<br class="">> ===================================================================<br class="">> --- /dev/null<br class="">> +++ runtimes/CMakeLists.txt<br class="">> @@ -0,0 +1,16 @@<br class="">> +include(LLVMExternalProjectUtils)<br class="">> +<br class="">> +# Discover the projects that use CMake in the subdirectories.<br class="">> +# Note that explicit cmake invocation is required every time a new project is<br class="">> +# added or removed.<br class="">> +<br class="">> +add_custom_target(runtimes)<br class="">> +<br class="">> +file(GLOB entries *)<br class="">> +foreach(entry ${entries})<br class="">> + if(IS_DIRECTORY ${entry} AND EXISTS ${entry}/CMakeLists.txt)<br class="">> + get_filename_component(projName ${entry} NAME)<br class="">> + llvm_ExternalProject_Add(${projName} ${entry} USE_TOOLCHAIN)<br class="">> + add_dependencies(runtimes ${projName})<br class="">> + endif()<br class="">> +endforeach(entry)<br class="">> Index: cmake/modules/LLVMExternalProjectUtils.cmake<br class="">> ===================================================================<br class="">> --- cmake/modules/LLVMExternalProjectUtils.cmake<br class="">> +++ cmake/modules/LLVMExternalProjectUtils.cmake<br class="">> @@ -29,7 +29,8 @@<br class="">> # Extra targets in the subproject to generate targets for<br class="">> # )<br class="">> function(llvm_ExternalProject_Add name source_dir)<br class="">> - cmake_parse_arguments(ARG "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL"<br class="">> + cmake_parse_arguments(ARG<br class="">> + "USE_TOOLCHAIN;EXCLUDE_FROM_ALL;NO_INSTALL;ALWAYS_CLEAN"<br class="">> "SOURCE_DIR"<br class="">> "CMAKE_ARGS;TOOLCHAIN_TOOLS;RUNTIME_LIBRARIES;DEPENDS;EXTRA_TARGETS" ${ARGN})<br class="">> canonicalize_tool_name(${name} nameCanon)<br class="">> @@ -52,6 +53,10 @@<br class="">> endif()<br class="">> endforeach()<br class="">><br class="">> + if(ARG_ALWAYS_CLEAN)<br class="">> + set(always_clean clean)<br class="">> + endif()<br class="">> +<br class="">> list(FIND TOOLCHAIN_TOOLS clang FOUND_CLANG)<br class="">> if(FOUND_CLANG GREATER -1)<br class="">> set(CLANG_IN_TOOLCHAIN On)<br class="">> @@ -135,6 +140,14 @@<br class="">> CMAKE_ARGS ${${nameCanon}_CMAKE_ARGS}<br class="">> ${compiler_args}<br class="">> -DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}<br class="">> + -DLLVM_CONFIG_PATH=${LLVM_RUNTIME_OUTPUT_INTDIR}/llvm-config<br class="">> + -DLLVM_LIBRARY_OUTPUT_INTDIR=${LLVM_LIBRARY_OUTPUT_INTDIR}<br class="">> + -DLLVM_RUNTIME_OUTPUT_INTDIR=${LLVM_RUNTIME_OUTPUT_INTDIR}<br class="">> + -DLLVM_LIBDIR_SUFFIX=${LLVM_LIBDIR_SUFFIX}<br class="">> + -DLLVM_ENABLE_WERROR=${LLVM_ENABLE_WERROR}<br class="">> + -DPACKAGE_VERSION=${PACKAGE_VERSION}<br class="">> + -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}<br class="">> + -DCMAKE_MAKE_PROGRAM=${CMAKE_MAKE_PROGRAM}<br class=""><br class="">How did you decide which variables need to be passed through like this?<br class="">The set seems somewhat arbitrary, but I may be missing something<br class="">obvious.<br class=""><br class="">> ${ARG_CMAKE_ARGS}<br class="">> ${PASSTHROUGH_VARIABLES}<br class="">> INSTALL_COMMAND ""<br class="">> @@ -152,7 +165,7 @@<br class="">> ExternalProject_Add_Step(${name} force-rebuild<br class="">> COMMAND ${run_build}<br class="">> COMMENT "Forcing rebuild of ${name}"<br class="">> - DEPENDEES configure clean<br class="">> + DEPENDEES configure ${always_clean}<br class=""><br class="">I'm not sure I understand what this does. If I had to guess I'd say that<br class="">when ALWAYS_CLEAN is passed the rebuild of the external project always<br class="">invokes clean first, but if it's not passed we'll just invoke the<br class="">external build and allow it to be incremental if appropriate. Is that<br class="">right?<br class=""><br class="">> DEPENDS ${ALWAYS_REBUILD} ${ARG_DEPENDS} ${TOOLCHAIN_BINS}<br class="">> ${cmake_3_4_USES_TERMINAL} )<br class="">> endif()<br class="">> Index: CMakeLists.txt<br class="">> ===================================================================<br class="">> --- CMakeLists.txt<br class="">> +++ CMakeLists.txt<br class="">> @@ -720,6 +720,8 @@<br class="">> add_subdirectory(tools)<br class="">> endif()<br class="">><br class="">> +add_subdirectory(runtimes)<br class="">> +<br class="">> if( LLVM_INCLUDE_EXAMPLES )<br class="">> add_subdirectory(examples)<br class="">> endif()<br class="">> @@ -730,7 +732,8 @@<br class="">> llvm_ExternalProject_Add(test-suite ${LLVM_MAIN_SRC_DIR}/projects/test-suite<br class="">> USE_TOOLCHAIN<br class="">> EXCLUDE_FROM_ALL<br class="">> - NO_INSTALL)<br class="">> + NO_INSTALL<br class="">> + ALWAYS_CLEAN)<br class="">> endif()<br class="">> add_subdirectory(test)<br class="">> add_subdirectory(unittests)<br class="">><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></blockquote></div></div></div></blockquote></div><br class=""></div></body></html>