[PATCH] D92347: [CSSPGO] Consume pseudo-probe-based AutoFDO profile

[Hongtao Yu via Phabricator via llvm-commits]

hoy marked 2 inline comments as done.
hoy added inline comments.


================
Comment at: llvm/lib/IR/PseudoProbe.cpp:24
+Optional<PseudoProbe> extractProbeFromDiscriminator(const Instruction &Inst) {
+  assert(isa<CallBase>(&Inst) && !isa<IntrinsicInst>(&Inst));
+  if (const DebugLoc &DLoc = Inst.getDebugLoc()) {
----------------
wmi wrote:
> Add a message for it.
Done.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:46
+          mdconst::dyn_extract<ConstantInt>(MD->getOperand(1))->getZExtValue();
+      GUIDToProbeDescMap.insert({GUID, PseudoProbeDescriptor(GUID, Hash)});
+    }
----------------
wmi wrote:
> Change to emplace_back to avoid build issue of old GCC?
Good catch, thanks!


================
Comment at: llvm/test/Transforms/SampleProfile/pseudo-probe-profile.ll:1
+; RUN: opt < %s -passes=pseudo-probe,sample-profile -sample-profile-file=%S/Inputs/pseudo-probe-profile.prof -pass-remarks=sample-profile -pass-remarks-output=%t.opt.yaml -S | FileCheck %s
+; RUN: FileCheck %s -check-prefix=YAML < %t.opt.yaml
----------------
wmi wrote:
> Could you also add a test similar to test/Transforms/SampleProfile/function_metadata.ll? That is to verify pseudo probe works with thinlto -- hot inline instance will be added to function_entry_count metadata so that thinlto can import them. 
Thanks for the suggestion. Adjusted `function_metadata.ll` for pseudo probe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92347