[PATCH] D36351: [lld][ELF] Add profile guided section layout

[Rafael Ávila de Espíndola via Phabricator via llvm-commits]

rafael added a comment.

Michael Spencer via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> @@ -1110,6 +1111,14 @@
> 
>   // If no layout was provided by linker script, we want to apply default
>   // sorting for special input sections. This also handles --symbol-ordering-file.
>   template <class ELFT> void Writer<ELFT>::sortInputSections() {
> 
> +  // Use the rarely used option -call-graph-ordering-file to sort sections.
>  +  if (!Config->CallGraphProfile.empty()) {
>  +    DenseMap<const InputSectionBase *, int> Order =
>  +        computeCallGraphProfileOrder();
>  +    for (BaseCommand *Base : Script->SectionCommands)
>  +      if (auto *Sec = dyn_cast<OutputSection>(Base))
>  +        sortSection(Sec, Order);
>  +  }

Instead of adding this if you can change buildSectionOrder:

static DenseMap<const InputSectionBase *, int> buildSectionOrder() {

  if (!Config->CallGraphProfile.empty())
    return computeCallGraphProfileOrder();
  ...

> +    InputSectionBase *FromSec = SymbolSection.lookup(Fields[0]);
>  +    InputSectionBase *ToSec = SymbolSection.lookup(Fields[1]);
>  +    if (FromSec && ToSec)
>  +      Config->CallGraphProfile[std::make_pair(FromSec, ToSec)] = Count;

This should be "+=", no? If there are multiple calls from one section to
another I would expect us to use the total in the graph.

> +struct EdgeDenseMapInfo {
>  +  static Edge getEmptyKey() {
>  +    return {DenseMapInfo<SectionIndex>::getEmptyKey(),
>  +            DenseMapInfo<SectionIndex>::getEmptyKey(), 0, 0};
>  +  }

This doesn't build with clang:

/home/espindola/llvm/llvm/tools/lld/ELF/CallGraphSort.cpp:115:12: error:
no matching constructor for initialization of '(anonymous
namespace)::Edge'

  return {DenseMapInfo<SectionIndex>::getEmptyKey(),
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

/home/espindola/llvm/llvm/tools/lld/ELF/CallGraphSort.cpp:104:8: note:
candidate constructor (the implicit copy constructor) not viable:
requires 1 argument, but 4 were provided
struct Edge {

  ^

/home/espindola/llvm/llvm/tools/lld/ELF/CallGraphSort.cpp:104:8: note:
candidate constructor (the implicit move constructor) not viable:
requires 1 argument, but 4 were provided
/home/espindola/llvm/llvm/tools/lld/ELF/CallGraphSort.cpp:104:8: note:
candidate constructor (the implicit default constructor) not viable:
requires 0 arguments, but 4 were provided

Cheers,
Rafael


https://reviews.llvm.org/D36351