[ELF] De-template LinkerDriver::link. NFC

[MaskRay]

Summary

Replace f<ELFT>(x) with InvokeELFT(f, x).
The size reduction comes from turning link from 4 specializations into 1.

My x86-64 lld executable is 26KiB smaller.

[MaskRay]

Move invokeELFT to Target.h to be used by other files

[lichray]

@rsmith, your suggestion in 2017 solves this issue:

template<typename T> void f();
template<template<class> auto F> auto invokeELFT() { ... }

invokeELFT<f>();

Works for PMFs as well.

[lichray]

LGTM

lld/ELF/Target.h

302 ↗

(On Diff #404342)

Should this be using push and pop?

[ikudrin]

LGTM

lld/ELF/Target.h

319 ↗

(On Diff #404342)

Why not preserve "unknown config->ekind"?

[mstorsjo]

This causes a bunch of build warnings for me, like this:

../tools/lld/ELF/InputSection.cpp:86:37: warning: ISO C++11 requires at least one argument for the "..." in a variadic macro  
   86 |     invokeELFT(parseCompressedHeader);
      |                                     ^

[MaskRay]

In D118551#3288226, @mstorsjo wrote:

This causes a bunch of build warnings for me, like this:

../tools/lld/ELF/InputSection.cpp:86:37: warning: ISO C++11 requires at least one argument for the "..." in a variadic macro  
   86 |     invokeELFT(parseCompressedHeader);
      |                                     ^

What's your compiler? It may be the -pedantic option.

This is supported since C++20 but we are compiling in -std=c++14 mode now.

For older clang, I used #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" to remove the diagnostic.

lld/ELF/Target.h

319 ↗

(On Diff #404342)

Will update this message.

[mstorsjo]

In D118551#3288308, @MaskRay wrote:

In D118551#3288226, @mstorsjo wrote:

This causes a bunch of build warnings for me, like this:

../tools/lld/ELF/InputSection.cpp:86:37: warning: ISO C++11 requires at least one argument for the "..." in a variadic macro  
   86 |     invokeELFT(parseCompressedHeader);
      |                                     ^

What's your compiler? It may be the -pedantic option.

This is supported since C++20 but we are compiling in -std=c++14 mode now.

For older clang, I used #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" to remove the diagnostic.

This is with GCC 9.3 (Ubuntu 20.04 default compiler).

[MaskRay]

In D118551#3288334, @mstorsjo wrote:

In D118551#3288308, @MaskRay wrote:

In D118551#3288226, @mstorsjo wrote:

This causes a bunch of build warnings for me, like this:

../tools/lld/ELF/InputSection.cpp:86:37: warning: ISO C++11 requires at least one argument for the "..." in a variadic macro  
   86 |     invokeELFT(parseCompressedHeader);
      |                                     ^

What's your compiler? It may be the -pedantic option.

This is supported since C++20 but we are compiling in -std=c++14 mode now.

For older clang, I used #pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments" to remove the diagnostic.

This is with GCC 9.3 (Ubuntu 20.04 default compiler).

In GCC the diagnostic is controlled by -pedantic/-Wpedantic.
At least 11 correctly suppresses the warning in -stdc=c++2a mode.

Unfortunately, for older language modes, #pragma GCC diagnostic ignored "-Wpedantic" does not suppress the diagnostic (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90843).
We can either ignore it (GCC compiling llvm-project has many diagnostics anyway) or change llvm/CMakeLists.txt to default LLVM_ENABLE_PEDANTIC to off for GCC.

[MaskRay]

lld/ELF/Target.h

302 ↗

(On Diff #404342)

Perhaps not worth the trouble. This usage is allowed in -std=c++20 mode...

[mstorsjo]

In D118551#3288646, @MaskRay wrote:

In GCC the diagnostic is controlled by -pedantic/-Wpedantic.
At least 11 correctly suppresses the warning in -stdc=c++2a mode.

Unfortunately, for older language modes, #pragma GCC diagnostic ignored "-Wpedantic" does not suppress the diagnostic (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90843).

Hmm, that's a shame...

We can either ignore it (GCC compiling llvm-project has many diagnostics anyway)

Not that many overall actually - building llvm+clang+lld+lldb normally only prints a couple (<10?) warnings - I try to get rid of new warnings when I notice them, if they're easily fixable... This added 7 new ones.

or change llvm/CMakeLists.txt to default LLVM_ENABLE_PEDANTIC to off for GCC.

Hmm, maybe that would a reasonable compromise, as the warning-cleanliness of the codebase isn't kept quite as strict with GCC anyway.

lld/ELF/Driver.h

 public:
   void linkerMain(ArrayRef<const char *> args);
   void addFile(StringRef path, bool withLOption);
   void addLibrary(StringRef name);
 
 private:
   void createFiles(llvm::opt::InputArgList &args);
   void inferMachineType();
-  template <class ELFT> void link(llvm::opt::InputArgList &args);
+  void link(llvm::opt::InputArgList &args);
   template <class ELFT> void compileBitcodeFiles(bool skipLinkedOutput);
 
   // True if we are in --whole-archive and --no-whole-archive.
   bool inWholeArchive = false;
 
   // True if we are in --start-lib and --end-lib.
   bool inLib = false;
 

lld/ELF/Driver.cpp

if (config->timeTraceEnabled)
     if (errorCount())
       return;
 
     // The Target instance handles target-specific stuff, such as applying
     // relocations or writing a PLT section. It also contains target-dependent
     // values such as a default image base address.
     target = getTarget();
 
-    switch (config->ekind) {
-    case ELF32LEKind:
-      link<ELF32LE>(args);
-      break;
-    case ELF32BEKind:
-      link<ELF32BE>(args);
-      break;
-    case ELF64LEKind:
-      link<ELF64LE>(args);
-      break;
-    case ELF64BEKind:
-      link<ELF64BE>(args);
-      break;
-    default:
-      llvm_unreachable("unknown Config->EKind");
-    }
+    link(args);
   }
 
   if (config->timeTraceEnabled) {
     checkError(timeTraceProfilerWrite(
         args.getLastArgValue(OPT_time_trace_file_eq).str(),
         config->outputFile));
     timeTraceProfilerCleanup();
   }
static uint32_t getAndFeatures() {
 
   // Force enable Shadow Stack.
   if (config->zShstk)
     ret |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
 
   return ret;
 }
 
+#ifdef __clang__
+#pragma clang diagnostic ignored "-Wgnu-zero-variadic-macro-arguments"
+#endif
+#define invokeELFT(f, ...)                                                     \
+  switch (config->ekind) {                                                     \
+  case ELF32LEKind:                                                            \
+    f<ELF32LE>(__VA_ARGS__);                                                   \
+    break;                                                                     \
+  case ELF32BEKind:                                                            \
+    f<ELF32BE>(__VA_ARGS__);                                                   \
+    break;                                                                     \
+  case ELF64LEKind:                                                            \
+    f<ELF64LE>(__VA_ARGS__);                                                   \
+    break;                                                                     \
+  case ELF64BEKind:                                                            \
+    f<ELF64BE>(__VA_ARGS__);                                                   \
+    break;                                                                     \
+  default:                                                                     \
+    llvm_unreachable("");                                                      \
+  }
+
 // Do actual linking. Note that when this function is called,
 // all linker scripts have already been parsed.
-template <class ELFT> void LinkerDriver::link(opt::InputArgList &args) {
+void LinkerDriver::link(opt::InputArgList &args) {
   llvm::TimeTraceScope timeScope("Link", StringRef("LinkerDriver::Link"));
   // If a --hash-style option was not given, set to a default value,
   // which varies depending on the target.
   if (!args.hasArg(OPT_hash_style)) {
     if (config->emachine == EM_MIPS)
       config->sysvHash = true;
     else
       config->sysvHash = config->gnuHash = true;
const bool skipLinkedOutput = config->thinLTOIndexOnly || config->emitLLVM ||
                                 config->ltoEmitAsm ||
                                 !config->thinLTOModulesToCompile.empty();
 
   // Do link-time optimization if given files are LLVM bitcode files.
   // This compiles bitcode files into real object files.
   //
   // With this the symbol table should be complete. After this, no new names
   // except a few linker-synthesized ones will be added to the symbol table.
-  compileBitcodeFiles<ELFT>(skipLinkedOutput);
+  invokeELFT(compileBitcodeFiles, skipLinkedOutput);
 
   // Symbol resolution finished. Report backward reference problems.
   reportBackrefs();
   if (errorCount())
     return;
 
   // Bail out if normal linked output is skipped due to LTO.
   if (skipLinkedOutput)
for (BinaryFile *f : binaryFiles)
       for (InputSectionBase *s : f->getSections())
         inputSections.push_back(cast<InputSection>(s));
   }
 
   {
     llvm::TimeTraceScope timeScope("Strip sections");
     llvm::erase_if(inputSections, [](InputSectionBase *s) {
       if (s->type == SHT_LLVM_SYMPART) {
-        readSymbolPartitionSection<ELFT>(s);
+        invokeELFT(readSymbolPartitionSection, s);
         return true;
       }
 
       // We do not want to emit debug sections if --strip-all
       // or --strip-debug are given.
       if (config->strip == StripPolicy::None)
         return false;
 
if (config->armHasBlx == false)
            "feature detected");
   }
 
   // This adds a .comment section containing a version string.
   if (!config->relocatable)
     inputSections.push_back(createCommentSection());
 
   // Split SHF_MERGE and .eh_frame sections into pieces in preparation for garbage collection.
-  splitSections<ELFT>();
+  invokeELFT(splitSections);
 
   // Garbage collection and removal of shared symbols from unused shared objects.
-  markLive<ELFT>();
+  invokeELFT(markLive);
   demoteSharedSymbols();
 
   // Make copies of any input sections that need to be copied into each
   // partition.
   copySectionsIntoPartitions();
 
   // Create synthesized sections such as .got and .plt. This is called before
   // processSectionCommands() so that they can be placed by SECTIONS commands.
-  createSyntheticSections<ELFT>();
+  invokeELFT(createSyntheticSections);
 
   // Some input sections that are used for exception handling need to be moved
   // into synthetic sections. Do that now so that they aren't assigned to
   // output sections in the usual way.
   if (!config->relocatable)
     combineEhSections();
 
   {
if (!config->relocatable)
     llvm::erase_if(inputSections, [](InputSectionBase *s) {
       return isa<MergeInputSection>(s);
     });
   }
 
   // Two input sections with different output sections should not be folded.
   // ICF runs after processSectionCommands() so that we know the output sections.
   if (config->icf != ICFLevel::None) {
-    findKeepUniqueSections<ELFT>(args);
-    doIcf<ELFT>();
+    invokeELFT(findKeepUniqueSections, args);
+    invokeELFT(doIcf);
   }
 
   // Read the callgraph now that we know what was gced or icfed
   if (config->callGraphProfileSort) {
     if (auto *arg = args.getLastArg(OPT_call_graph_ordering_file))
       if (Optional<MemoryBufferRef> buffer = readFile(arg->getValue()))
         readCallGraph(*buffer);
-    readCallGraphsFromObjectFiles<ELFT>();
+    invokeELFT(readCallGraphsFromObjectFiles);
   }
 
   // Write the result to the file.
-  writeResult<ELFT>();
+  invokeELFT(writeResult);
 }