[PATCH] D117598: [llvm-profdata] Use SubCommands to simplify code

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:36
     "profile-symbol-list-cutoff", cl::Hidden, cl::init(-1), cl::ZeroOrMore,
+    cl::sub(*cl::TopLevelSubCommand), cl::sub(llvm_profdata::MergeSubCommand),
+    cl::sub(llvm_profdata::ShowSubCommand),
----------------
wenlei wrote:
> This creates a dependency from profile data to llvm-profdata, and ideally should be avoided. 
> 
> Same for other changes in ProfileData
Yeah, I kind of agree. I think there is a lot of value in this patch, but these options caused a lot of trouble. This patch was the cleanest solution I could come up with.

IMO there already is a dependency between SampleProf and llvm-profdata. The `llvm-profdata` tool calls into SampleProf and reads a global option (`profile-symbol-list-cutoff`). When I switched to using SubCommands, the global option was no longer recognized so I had to add the subcommands here. It might be better to add these as arguments to `SampleProfileReader::create()`, but that might require redundant tool options since multiple tool use this. Also this would require significant refactoring since there are several options like this.

This patch isn't necessary, but it does make the `llvm-profdata` tool easier to maintain and use and I'd like to find some way to do this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D117598/new/

https://reviews.llvm.org/D117598