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

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:103
+  SPL_Nest = 0x1,
+  SPL_CS = 0x2,
+};
----------------
I think flat vs nest is better here (than cs vs nest).


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:375-376
+      CallTargets.erase(I);
+      if (*Count <= NumSamples)
+        NumSamples -= *Count;
+      else {
----------------
addCalledTarget does not add to NumSamples, but removeCalledTarget subtracts NumSamples. What's the reason for the inconsistency? 


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:517
+    if (!PreserveContextAttributes)
+      ChildProfile->getContext().setAllAttributes(0);
+
----------------
nit: 0->ContextNone


================
Comment at: llvm/lib/ProfileData/SampleProf.cpp:529-530
+      // weight.
+      NodeProfile->removeCalledTargetAndUpdateTotalSamples(
+          ChildNode.CallSiteLoc.LineOffset, ChildNode.CallSiteLoc.Discriminator, OrigChildContext.getName());
     }
----------------
It looks like the discrepancy comes from the fact that flat profile has target counts included in total samples, but nest profile doesn't? Is this something that should be unified? I don't remember why we choose to include target counts (rather than SampleRecord::NumSamples) in total counts for flat profile. Was it intentional?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121651