[PATCH] D91756: [CSSPGO] Pseudo probes for function calls.
Hongtao Yu via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 1 16:04:26 PST 2020
hoy added inline comments.
================
Comment at: llvm/lib/CodeGen/PseudoProbeInserter.cpp:50
+ for (MachineBasicBlock &MBB : MF) {
+ MachineInstr *FirstInstr = nullptr;
+ for (MachineInstr &MI : MBB) {
----------------
wmi wrote:
> What is the usage of FirstInstr?
Good patch. It's not used in this change. It's used in an upcoming change where a probe probing an empty block will be marked specially for better counts inference.
================
Comment at: llvm/lib/CodeGen/PseudoProbeInserter.cpp:54
+ FirstInstr = &MI;
+ if (MI.isCall()) {
+ if (DILocation *DL = MI.getDebugLoc()) {
----------------
wmi wrote:
> Will tailcall or other optimizations convert call into something else before PseudoProbeInserter pass?
Good point. A tailcall instruction (a special MIR jump instruction like `TAILJMPd`) is considered as a call and can be identified as `MI.isCall() && MI.isTerminator() && MI.isReturn()`. Tail calls are bad to the frame-pointer-based virtual unwinding since they may cause missing frames. An upcoming change will mark them specially and a tail call tracker in `llvm-profgen` will try to track the missing frames based on some static analyses.
================
Comment at: llvm/lib/Transforms/IPO/SampleProfileProbe.cpp:136-138
+ for (auto &I : CallProbeIds) {
+ auto Call = I.first;
+ uint32_t Index = I.second;
----------------
wmi wrote:
> for (auto &[Call, Index] : CallProbeIds) {
Thanks for the suggestion. This is a C++17 usage and may cause a warning with the current build setup which defaults to C++14 (due to -Wc++17)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91756/new/
https://reviews.llvm.org/D91756
More information about the llvm-commits
mailing list