[PATCH] D89723: [CSSPGO][llvm-profgen]Context-sensitive profile data generation

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:184
+
+bool PerfReader::extractLBRStack(line_iterator &LBRLine, UnwinderTrace &Trace) {
+  SmallVector<StringRef, 32> Records;
----------------
Can you give an example of LBRStack so it is easy to understand what the code is parsing here?


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:219-238
+    if (!SrcIsInternal && !DstIsInternal) {
+      // Ignore branches outside the current binary.
+      continue;
+    } else if (!SrcIsInternal && DstIsInternal) {
+      // For transition from external code (such as dynamic libraries) to
+      // the current binary, keep track of the branch target which will be
+      // grouped with the Source of the last transition from the current
----------------
Removing the else if make the code a little easier to read.
```
if (!SrcIsInternal && !DstIsInternal) 
  continue;

if (!SrcIsInternal && DstIsInternal) {
  PrevTrDst = Dst;
  continue;
}

if (SrcIsInternal && !DstIsInternal) {
  if (!PrevTrDst)
    continue;
  Dst = PrevTrDst;
  PrevTrDst = 0;
  IsArtificial = true;
}

// Filter out the branch sample
...
```


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:265
+
+bool PerfReader::extractCallstack(line_iterator &Line, UnwinderTrace &Trace) {
+  // Extract stack frames from sample group
----------------
Same as above, better to give an example showing what the function is parsing here.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:308-310
+  // Filter out broken stack sample. We may not have complete frame info
+  // if sample end up in prolog/epilog, the result is dangling context not
+  // connected to entry point. This should be relatively rare thus not much
----------------
What sampling event are you using? If br_inst_retired:near_taken is used, the sample won't end up in prolog/epilog.


================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.h:23
+
+class ProfileGenerator {
+
----------------
I thought the tool can also generate profile for current debug info based non CS AFDO but I am not sure. I guess that is a special case handled by CSProfileGenerator. Could you confirm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89723