[PATCH] D53683: [ELF] Add --{, no-}call-graph-profile (enabled by default)

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Driver.cpp:789
   Config->EmitRelocs = Args.hasArg(OPT_emit_relocs);
+  Config->EnableCallGraphProfile =
+      Args.hasFlag(OPT_call_graph_profile, OPT_no_call_graph_profile, true);
----------------
MaskRay wrote:
> MaskRay wrote:
> > ruiu wrote:
> > > nit: you probably should spend a bit more time to read existing code around the location where you add new code to make your new code consistent with existing one. All the other `Config` members have the exact same name with a corresponding command line flag. For example, `Config->EhFrameHdr` corresponds to `--eh-frame-hdr` and `Config->EmitRelocs` corresponds to `--emit-relocs`. Please make your new code consistent with existing code.
> > I know, but the variable `CallGraphProfile` is already used as a map, what should I rename it to?
> 
> ```
> bool CallGraphProfile;
> llvm::MapVector<std::pair<const InputSectionBase *, const InputSectionBase *>,
>                   uint64_t>
>     CallGraphInfo;
> ```
> 
> Let me know if you have other preference
Yeah you should, because you essentially should keep code beautiful/readable for those who read this code for the first time. If in doubt, forget about history of code and see new code with fresh eye. I'm sure you'll find this code inconsistent and odd.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D53683