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

Greg Fitzgerald garious at gmail.com
Sat Feb 21 10:34:58 PST 2015


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.   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 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/






More information about the llvm-commits mailing list