This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by MaskRay on Jan 29 2022, 8:51 PM.

Details

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.

Diff Detail

Event Timeline

MaskRay created this revision.Jan 29 2022, 8:51 PM
MaskRay requested review of this revision.Jan 29 2022, 8:51 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 29 2022, 8:51 PM
MaskRay updated this revision to Diff 404342.Jan 29 2022, 10:43 PM
MaskRay edited the summary of this revision. (Show Details)

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

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

LGTM

lld/ELF/Target.h
302

Should this be using push and pop?

ikudrin accepted this revision.Feb 1 2022, 4:49 AM

LGTM

lld/ELF/Target.h
319

Why not preserve "unknown config->ekind"?

This revision is now accepted and ready to land.Feb 1 2022, 4:49 AM
This revision was automatically updated to reflect the committed changes.
MaskRay marked an inline comment as done.

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);
      |                                     ^

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

Will update this message.

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

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 added inline comments.Feb 1 2022, 1:22 PM
lld/ELF/Target.h
302

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

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.