[PATCH] D96434: [CSSPGO][llvm-profgen] Filter out the instructions without location info for symbolizer

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:98
   KeyStr->Context = Binary->getExpandedContextStr(Stack);
+  if (KeyStr->Context.empty())
+    return nullptr;
----------------
hoy wrote:
> Can we not return null here and instead just check if the return value has empty string below? 
This is a polymorphism function, for the callee the base class's not straightforward to get the `Context` string value. Or make a virtual function `isEmpty` for them?


================
Comment at: llvm/tools/llvm-profgen/ProfiledBinary.cpp:158
     const FrameLocationStack &ExpandedContext = getFrameLocationStack(Offset);
+    if (ExpandedContext.empty())
+      return "";
----------------
hoy wrote:
> Add comment for this? An instruction without a valid debug line will be ignored by sample processing.
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96434