[PATCH] D121862: [ProfSampleLoader] When disable-sample-loader-inlining is true, merge profiles of inlined instances to outlining versions.

[Mingming Liu via Phabricator via llvm-commits]

luna added a comment.

thanks for the reviews!



================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:188-194
+    cl::desc("If true, artifically skip inline transformation in sample-loader "
+             "pass, and merge (or scale) profiles (as configured by "
+             "--sample-profile-merge-inlinee). "
+             "Since profiles are consumed by many passes, turning on this "
+             "option has side effects."
+             "For instance, pre-link SCC inliner would see merged profiles and "
+             "inline the hot functions (that are skipped in this pass)."));
----------------
wenlei wrote:
> This seems too verbose - it's the longest message among all flags here. But I don't have a strong opinion. 
I agree.

Took the liberty to move the side-effects part into a "//" comment.

`--sample-profile-remapping-file` (in the same cpp file) has a comment [1] besides option description, so hopefully this is fine for code style.

[1] https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/IPO/SampleProfile.cpp#L123-L125


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:196
+
+// D121862
 cl::opt<int> ProfileInlineGrowthLimit(
----------------
wenlei wrote:
> remove
thanks for the catch! Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121862