[llvm-dev] [RFC] LLVM Directory Structure Changes (was Re: [PATCH] D20992: [CMake] Add LLVM runtimes directory)

Thu Jun 9 10:36:17 PDT 2016

----- Original Message -----
> From: "Chris Bieneman via llvm-dev" <llvm-dev at lists.llvm.org>
> To: "Chandler Carruth" <chandlerc at gmail.com>
> Cc: "llvm-dev" <llvm-dev at lists.llvm.org>
> Sent: Thursday, June 9, 2016 12:20:36 PM
> Subject: [llvm-dev] [RFC] LLVM Directory Structure Changes (was Re: [PATCH] D20992: [CMake] Add LLVM runtimes
> directory)
> 
> 
> 
> Moving to llvm-dev (I think this has gone a bit further than a patch
> review discussion)
> 
> 
> 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.
> 
> 
> 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:
> 
> 
> (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)
> (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)
> (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.
> 
> 
> 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.
> 
> 
> (1) Projects that are configured with LLVM
> (2) Runtime projects that should be configured using the just-built
> tools
> (3) The LLVM test-suite, which is really just external tests that
> should be configured and run with the just-built tools
> 
> 
> 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.
> 
> 
> 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.
> 
> 
> 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.

This all makes sense to me. It matches my mental model that tools get compiled with the 'host' compiler and the runtimes need to get cross-compiled. Putting the test-suite at the top-level could work, we already have a 'test' and 'unittests' directory. Maybe, however, unit tests should live under tests?

 -Hal

> 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.
> 
> 
> If we want to go with this proposal I envision the transition being
> multi-staged:
> 
> 
> (1) Adding the new functionality, getting it up and fully working for
> all runtime projects - this will involve changes to runtime projects
> (2) Work with bot maintainers to migrate bots, and fix any issues
> that come up
> (3) Add support for a new secondary location for the test-suite
> (4) Set a date for removing the projects directory, post patches
> including updated documentation
> (5) Remove the projects directory entirely
> 
> 
> Thoughts?
> -Chris
> 
> 
> 
> 
> 
> On Jun 8, 2016, at 6:59 PM, Chandler Carruth < chandlerc at gmail.com >
> wrote:
> 
> 
> 
> 
> On Wed, Jun 8, 2016 at 4:39 PM Justin Bogner via llvm-commits <
> llvm-commits at lists.llvm.org > wrote:
> 
> 
> 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?
> 
> 
> 
> I share your concerns, but I also kind of like the direction this is
> going.
> 
> 
> But there is a higher-level meta-point: do we want to keep the
> 'projects' directory *at all*.
> 
> 
> Every single resident of it I can think of except for the test-suite
> is either dead (dragonegg) or a runtime library.
> 
> 
> 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).
> 
> 
> 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.
> 
> 
> -Chandler
> 
> 
> 
> > 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)
> > 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory