[PATCH] D98823: [CSSPGO] Add attribute metadata for context profile

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:91
 ///     !CFGChecksum: 12345
 /// Stores the FunctionHash (a.k.a. CFG Checksum) into \p FunctionHash.
+static bool parseMetadata(const StringRef &Input, uint64_t &FunctionHash,
----------------
hoy wrote:
> Please update comment here and also the format comment at the beginning of SampleProfReader.h .
Updated.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:556
         FunctionSamples &CallerProfile =
-            getFunctionProfileForContext(CallerContextId);
+            getFunctionProfileForContext(CallerContextId, true);
         CallerProfile.setFunctionHash(InlinerDesc->FuncHash);
----------------
hoy wrote:
> Should caller profile be marked inlined? The callee was inlined into the caller, but the caller may not be inlined into caller's caller.
Good catch, you're right. 


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:630
+  return getFunctionProfileForLeafProbe(ContextStrStackCopy, FuncDesc,
+                                        WasLeafInlined);
 }
----------------
hoy wrote:
> I'm wondering this should also be done for non-probe CS profile generation.
Yeah, this is a TODO as mentioned in the change description. The change to support that for non-probe profile will actually be bigger since we put everything into flat context string out of unwinder, so wanted to do them separately. Now I left a TODO comment as well, but let me take another look.. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98823