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

[Michael Spencer via Phabricator via llvm-commits]

Bigcheese marked 3 inline comments as done.
Bigcheese added inline comments.


================
Comment at: ELF/CallGraphSort.cpp:57
+using ClusterIndex = std::ptrdiff_t;
+using SectionIndex = std::ptrdiff_t;
+using EdgeIndex = std::ptrdiff_t;
----------------
espindola wrote:
> Why do you need a 64 bit index. Why does it need a typedef?
While ELF limits the number of sections/symbols a single ELF file can hold, there's no limit on the number of input sections that go into the final link.  If we have multiple ELF files with 4bn sections, then this could overflow.  The chances of this are rare, but there's almost no cost in supporting it.  The correct default type for array indexing is std::ptrdiff_t, you need a good reason to use something else.

As for the typedef, it makes it clearer what the purpose of the index is. Otherwise they would all have the same type.  If C++ had a good way to do strong typedefs I would be using that here to add compile time enforcement.


================
Comment at: ELF/Driver.cpp:600
+    if (FromSym && ToSym)
+      Config->CallGraphProfile[std::make_pair(FromSym, ToSym)] += Count;
+  }
----------------
espindola wrote:
> This should now be just =, no?
It should be a saturating add.  An input file can legitimately have multiple edges, and they should be merged.


https://reviews.llvm.org/D36351