[PATCH] [lld] Remove duplicate class definitions of ELF LinkingContexts

[Hal Finkel]

----- Original Message -----
> From: "Greg Fitzgerald" <garious at gmail.com>
> To: garious at gmail.com, bigcheesegs at gmail.com
> Cc: "shankar kalpathi easwaran" <shankar.kalpathi.easwaran at gmail.com>, "filcab+llvm phabricator"
> <filcab+llvm.phabricator at gmail.com>, llvm-commits at cs.uiuc.edu
> Sent: Saturday, February 21, 2015 12:34:58 PM
> Subject: Re: [PATCH] [lld] Remove duplicate class definitions of ELF	LinkingContexts
> 
> LLD is in an awkward limbo state where it acts like it's a
> first-class member of the LLVM tree and also acts like an
> independent project that only depends on LLVM for its Support and
> CMake libraries.  If it wants to be the former, LLD should respect
> LLVM_TARGETS_TO_BUILD.  If the latter, it doesn't matter and could
> have its own Targets.def.  But if that's the case, LLD should have a
> standalone CMake build.
> 
> My preference *was* to treat LLD as if it were part of the LLVM tree,
> with the hope that LLD would be integrated. 

My understanding, as a general member of the LLVM community, was that the eventual goal was the integrate lld into the rest of the system. For a long time, this was not possible because lld used C++11 while LLVM itself did not. This is obviously not true any more, and I think that if there are further reasons to continue the current level of separation, those should be well explained.

>  I like the idea that
> the LLVM repo would be responsible for everything from bitcode to
> linked executables instead of everything from bitcode to object
> files. 

I also like this idea.

 -Hal

> I sent an email to llvmdev to see if others felt the same
> way, but got very little feedback.  So now I'm moving under the
> assumption that LLD will be a standalone project for the foreseeable
> future.  Surprisingly, my attempt to revive the standalone CMake
> build is being ignored.  Clarification would be much appreciated.
> 
> 
> REPOSITORY
>   rL LLVM
> 
> ================
> Comment at: lib/Driver/GnuLdDriver.cpp:339
> @@ -338,2 +338,3 @@
>    std::unique_ptr<ELFLinkingContext> p;
>    // FIXME: #include "llvm/Config/Targets.def"
> +#define LLVM_TARGET(name) \
> ----------------
> filcab wrote:
> > IIUC, nothing in lld depends on llvm's enabled targets (except some
> > MIPS tests, IIRC). Why wouldn't this just depend on “targets lld
> > can handle”?
> Because it could be both "targets lld can handle" and "targets
> enabled by the llvm build".  You can mix the two by adding a
> 'create' function for each llvm-supported target and returning
> nullptr to indicate "unsupported by lld".
> 
> ================
> Comment at: lib/Driver/GnuLdDriver.cpp:341
> @@ -342,1 +340,3 @@
> +#define LLVM_TARGET(name) \
> +  if ((p = elf::create##name##LinkingContext(triple))) return p;
>    LLVM_TARGET(AArch64)
> ----------------
> filcab wrote:
> > Why not switch on the Triple, and call the appropriate function
> > (which wouldn't have the Triple check, but (possibly) an assert),
> > instead of doing the whole “try to create*()” dance?
> So that we can enable only select targets via LLVM_TARGETS_TO_BUILD.
> 
> ================
> Comment at: lib/Driver/GnuLdDriver.cpp:344
> @@ -343,2 +343,2 @@
>    LLVM_TARGET(ARM)
>    LLVM_TARGET(Hexagon)
> ----------------
> filcab wrote:
> > We have this list of LLVM_TARGET expansions at least twice. Do we
> > want something like a .def file?
> Sure, I'll add a .def file.
> 
> http://reviews.llvm.org/D7807
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

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