[PATCH] D108433: [CSSPGO] split context string - compiler changes

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:420
+
+  bool hasLineLocation() const { return Callsite.LineOffset; }
+  void dropLineLocation() { Callsite = LineLocation(0, 0); }
----------------
hoy wrote:
> wenlei wrote:
> > LineOffset 0 can be a valid call site location given this is offset not absolute line number. 
> Good catch. Switched to using INT32_MAX as an invalid line offset.
On the second thought, sticking with LineOffset 0 for leaf frame is more natural. We have a bench places in the compiler/reader/writer/llvm-profgen that uses 0 as leaf line location. 

By default, `SampleContext` should represent a valid call context with the leaf frame, so most of the time we should just automatically strip off the leaf line location., such as when doing prefix check, getting the full context string. However there is a case in llvm-profgen where `SampleContext` is used to represent intermediate context for pseudo probes when the leaf probe is not ready. This causes diversion, especially in the full context string printing. Given that's the only place we diverge, I'm adding a flag to `getContextString` to tolerate that, instead of introducing the `hasLineLocation` API.

So with the latest change, by default, we should assume the last element of the context vector is the leaf frame, except in the special llvm-profgen case. Does that make sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108433