Page MenuHomePhabricator

[CSSPGO][llvm-profgen] Report samples for untrackable frames.
ClosedPublic

Authored by hoy on May 21 2021, 6:10 PM.

Details

Summary

Fixing an issue where samples collected for an untrackable frame is not reported. An untrackable frame refers to a frame whose caller is untrackable due to missing debug info or pseudo probe. Though the frame is connected to its parent frame through the frame pointer chain at runtime, the compiler cannot build the connection without debug info or pseudo probe. In such case we just need to report the untrackable frame as the base frame and all of its child frames.

With more samples reported I'm seeing this improves the performance of an internal benchmark by 2.5%.

Diff Detail

Event Timeline

hoy created this revision.May 21 2021, 6:10 PM
hoy requested review of this revision.May 21 2021, 6:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 21 2021, 6:10 PM
hoy edited the summary of this revision. (Show Details)May 21 2021, 6:14 PM
hoy added reviewers: wenlei, wlei, wmi, davidxl.
wlei added inline comments.May 24 2021, 10:23 AM
llvm/tools/llvm-profgen/PerfReader.cpp
150

Previously we always missed the context from the first frame, right? like foo @ bar, it will miss foo.

hoy added inline comments.May 24 2021, 10:37 AM
llvm/tools/llvm-profgen/PerfReader.cpp
150

Exactly. And if bar is inlined into foo, the issue will cause samples of bar to be ignored. For the attached regression test, without the fix, the output profile is empty.

wlei accepted this revision.May 24 2021, 10:40 AM

LGTM, thanks for adding useful debug printer.

llvm/tools/llvm-profgen/PerfReader.cpp
150

I see, thanks for the fix!!

This revision is now accepted and ready to land.May 24 2021, 10:40 AM
wenlei added inline comments.May 24 2021, 11:25 AM
llvm/test/tools/llvm-profgen/Inputs/truncated-pseudoprobe.ll
89

Is this the one missing probe id in discriminator?

llvm/tools/llvm-profgen/PerfReader.cpp
147

Why do we need this check? Is it ok for leaf to not find an associated call probe?

I'm thinking that the warning can be moved into ProbeStack::pushFrame, so it can be more specific, and the message can be "Truncated context due to missing call probe."

llvm/tools/llvm-profgen/PerfReader.h
611

is this removal intentional?

hoy added inline comments.May 24 2021, 12:12 PM
llvm/test/tools/llvm-profgen/Inputs/truncated-pseudoprobe.ll
89

Yes. The discriminator field is intentionally set empty to mimid the real case.

llvm/tools/llvm-profgen/PerfReader.cpp
147

isLeafFrame here means the address is from LBR not the stack samples. So it doesn't refer to a function frame and it'll mostly have no call probe associated.

Yes, moving the assert into ProbeStack::pushFrame sounds good.

llvm/tools/llvm-profgen/PerfReader.h
611

Yes, there's already a private preceding this.

hoy updated this revision to Diff 347472.May 24 2021, 12:19 PM

Addressing Wenlei's feebacks.

wenlei accepted this revision.May 24 2021, 12:24 PM
wenlei added inline comments.
llvm/tools/llvm-profgen/PerfReader.h
446

nit, mention "due to missing call probe" now that this is moved into ProbeStack.

lgtm with a nit, thanks for the fix.

hoy updated this revision to Diff 347473.May 24 2021, 12:28 PM

Addressing Wenlei's comment.

llvm/tools/llvm-profgen/PerfReader.h
446

Done. Sorry for missing this in your previous comment.

This revision was landed with ongoing or failed builds.May 24 2021, 12:39 PM
This revision was automatically updated to reflect the committed changes.