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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Config.h:113
+  llvm::DenseMap<std::pair<llvm::StringRef, llvm::StringRef>, uint64_t>
+      CGProfile;
   bool AllowMultipleDefinition;
----------------
Bigcheese wrote:
> 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.
I thought I set a clear standard there, but there are few variable names that I missed. My stance is that, if some word is used as one word in an option in a long-form command line option, I'd respect that and treat it as one word as a variable name. That helps me memorize command line option names and variable names. So please stick with the same hyphenation/camel case rule. (If you strongly feel that callgraph is not a word, you could name the option -call-graph-profile, but I wonder if you want to do this.)


https://reviews.llvm.org/D36351