[PATCH] D121651: [llvm-profdata] Convert nested profile to CS flat profile.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:545
 
-void CSProfileConverter::convertProfiles() { convertProfiles(RootFrame); }
+void CSProfileConverter::convertToCSNestedProfiles() {
+  for (auto &FuncSample : ProfileMap) {
----------------
wenlei wrote:
> Here the caller name is `convertToCSNestedProfiles`, but the callee on line 552 is `convertToCSFlatProfiles`.
Sorry, bad auto renaming.


================
Comment at: llvm/test/tools/llvm-profdata/cs-sample-flat-profile.test:1
+RUN: llvm-profdata merge --sample --text -output=%t.proftext %S/Inputs/sample-profile.proftext --gen-cs-flat-profile=1
+RUN: FileCheck %s < %t.proftext
----------------
wenlei wrote:
> Add round trip test for conversion? 
Done.


================
Comment at: llvm/tools/llvm-profdata/llvm-profdata.cpp:735
                    StringRef ProfileSymbolListFile, bool CompressAllSections,
-                   bool UseMD5, bool GenPartialProfile, bool GenCSNestedProfile,
+                   bool UseMD5, bool GenPartialProfile, bool GenCSNestedProfile, bool GenCSFlatProfile,
                    bool SampleMergeColdContext, bool SampleTrimColdContext,
----------------
wenlei wrote:
> Are the two flags (GenCSNestedProfile/GenCSFlatProfile and the corresponding switches) supposed to be mutually exclusive? If yes, perhaps we should make it explicit? 
> 
> one way to make then mutual exclusive is to use a single switch: gen-cs-nested-profile==true means nested, false means flat. Or we can have `-gen-cs-profile={nest|flat}`.
Introduced a --sample-profile-layout=cs|nest switch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121651