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

[Michael Spencer via Phabricator via llvm-commits]

Bigcheese added inline comments.


================
Comment at: ELF/Config.h:113
+  llvm::DenseMap<std::pair<llvm::StringRef, llvm::StringRef>, uint64_t>
+      CGProfile;
   bool AllowMultipleDefinition;
----------------
ruiu wrote:
> Bigcheese wrote:
> > ruiu wrote:
> > > Bigcheese wrote:
> > > > ruiu wrote:
> > > > > Bigcheese wrote:
> > > > > > ruiu wrote:
> > > > > > > Should be `CallgraphOrderingFile` as all Config members are named after their corresponding command line options.
> > > > > > That won't be the only way this member is updated, and it's not a file.
> > > > > Then can you just drop `File` and name this `CallgraphOrdering`?
> > > > It's also not an ordering. Which means the name of the option is wrong. Should be -callgraph-profile-file and `CallGraphProfile`.
> > > Please rename this CallgraphProfile as we generally have a member FoobarBaz in Config for option -foobar-baz.
> > There are only 2 that follow that, there are 3 that don't, and style guide says to use camel case and capitalize the same word consistently. Everywhere else in llvm it's CallGraph (1 match for Callgraph in a string, 1300 matches for CallGraph).
> What are the three options that don't follow? I need to fix them.
Follow:
* NoinhibitExec
* Nostdlib
* Omagic
* ZCombreloc
* ZNocopyreloc
* ZNodelete
* ZNodlopen
* ZRodynamic
* ZWxneeded
Don't follow:
* OFormatBinary
* SoName
* LTONewPmPasses
* ThinLTOCacheDir
* ThinLTOCachePolicy
* ThinLTOJobs

Given that there's no clear standard, I don't see why we would change names to disagree with the coding style used in the rest of LLVM rather than to agree with it.


https://reviews.llvm.org/D36351