[llvm-dev] [RFC] LLVM Directory Structure Changes (was Re: [PATCH] D20992: [CMake] Add LLVM runtimes directory)
Hal Finkel via llvm-dev
llvm-dev at lists.llvm.org
Thu Jun 9 11:36:47 PDT 2016
----- Original Message -----
> From: "Chris Bieneman via llvm-dev" <llvm-dev at lists.llvm.org>
> To: "Justin Bogner" <mail at justinbogner.com>
> Cc: "llvm-dev" <llvm-dev at lists.llvm.org>
> Sent: Thursday, June 9, 2016 1:23:18 PM
> Subject: Re: [llvm-dev] [RFC] LLVM Directory Structure Changes (was
> Re: [PATCH] D20992: [CMake] Add LLVM runtimes directory)
> > On Jun 9, 2016, at 10:49 AM, Justin Bogner < mail at justinbogner.com
> > >
> > wrote:
>
> > Chris Bieneman < beanz at apple.com > writes:
>
> > > 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 seems pretty sensible. Should we also use the opportunity
> > to
>
> > split compiler-rt's builtins and profiling/sanitizer/etc runtimes,
> > since
>
> > we'll be moving things around anyway?
>
> About that… So this is a complicated issue, but we should discuss it.
> Building compiler-rt as a monolithic chunk under the runtimes model
> is actually problematic because it would have circular dependencies.
> For example:
> libclang_rt.asan.aarch64 depends on libcxx.aarch64, but
> libcxx.aarch64 depends on libclang_rt.builtins.aarch64.
> That means to satisfy the proper build dependencies you need to build
> clang, then builtins, then libcxx, then the sanitizer libraries.
I think we might take this opportunity to break apart the various essentially-disjoint parts of compiler-rt. That should make this dependency problem much easier to handle.
-Hal
> Since Compiler-RT’s build does support configuring the builtins
> directory separately from the sanitizers, we could support this with
> my runtimes proposal without changing anything else in Compiler-RT
> through the application of some project-specific hacks. Not idea,
> but it means this doesn’t need to block my proposal.
> We should consider other alternatives, because it would nice to not
> have hacks. As an added benefit, if we separated the builtins and
> sanitizers into separate libraries we would be able to have the
> sanitizer libraries licensed the same way as LLVM (with the
> attribution clause), which would limit the amount of code that has
> its wonky modified license.
> -Chris
> > Some place like test/external or test/integration would probably
> > make
>
> > sense. It could potentially also be used for other optional tests
> > like
>
> > debuginfo-tests, which are currently somewhat awkwardly checked out
> > into
>
> > clang's tests.
>
> > > 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
> >
>
> > Sure, but we might as well update the documentation earlier (in
> > step
> > 1)
>
> > - as soon as compiler-rt can live in runtimes it makes sense to
> > tell
>
> > people to put it there, even if we still have legacy logic to make
> > it
>
> > continue to work out of projects as well.
>
> > > (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 <
> > > > mailto:llvm-commits at lists.llvm.org
> > > > >>
> > >
> >
>
> > > > 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?
> > >
> >
>
> > > > 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 <
> > > > > 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 <
> > > > mailto:llvm-commits at lists.llvm.org
> > > > >
> > >
> >
>
> > > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> > >
> >
>
> > > > < 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160609/5ffa96fb/attachment-0001.html>
More information about the llvm-dev
mailing list