[PATCH] D110859: [llvm-profgen] Support symbol list for accurate profile

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:35
 
+static cl::opt<bool> PopulateProfileSymbolList(
+    "populate-profile-symbol-list", cl::init(true), cl::Hidden,
----------------
hoy wrote:
> On the second thought, do we want the default value of `PopulateProfileSymbolList` to be true or false? We probably do not want the list for either CS or non-CS in practice for now.
I checked that our internal tools make it on by default for non-CS. But we haven't tested for CS profile.

Or we can make it off for CS util we tested it and then we can turn on?


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:101
+    for (const auto &Item : ProfileMap) {
+      const FunctionSamples Profile = Item.second;
+      SymbolList.add(Profile.getName(), true);
----------------
hoy wrote:
> Use const FunctionSamples& ?
Good catch, thanks!


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

https://reviews.llvm.org/D110859