This is an archive of the discontinued LLVM Phabricator instance.

[llvm-profgen] Ignore the whole trace with the leading external branch
ClosedPublic

Authored by wlei on Oct 13 2021, 11:21 AM.

Details

Summary

The first LBR entry can be an external branch, we should ignore the whole trace.

7f7448e889e4 0x7f7448e889e4/0x7f7448e88826/P/-/-/1  0x7f7448e8899f/0x7f7448e889d8/P/-/-/4  ...

Diff Detail

Event Timeline

wlei created this revision.Oct 13 2021, 11:21 AM
wlei requested review of this revision.Oct 13 2021, 11:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 13 2021, 11:21 AM
wlei edited the summary of this revision. (Show Details)Oct 13 2021, 11:29 AM
wlei added reviewers: wenlei, hoy.
hoy accepted this revision.Oct 13 2021, 11:46 AM

Good catch, thx.

This revision is now accepted and ready to land.Oct 13 2021, 11:46 AM
wenlei added inline comments.Oct 13 2021, 12:20 PM
llvm/tools/llvm-profgen/PerfReader.cpp
502

This means we always ignore any external branches, and we will no longer generate any artificial branches for internal->external->internal transition?

wlei updated this revision to Diff 379492.Oct 13 2021, 12:22 PM

Updating D111749: [llvm-profgen] Also ignore the first external branch

wenlei added inline comments.Oct 13 2021, 12:36 PM
llvm/tools/llvm-profgen/PerfReader.cpp
493

As we discussed, we shouldn't relax this for CS profile - it can't handle leading external address during unwinding.

wlei updated this revision to Diff 379551.Oct 13 2021, 4:26 PM
  1. revert back to preivous version, ignore the whole trace
  2. add test case for artificial branch
wenlei added inline comments.Oct 13 2021, 4:31 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
1–2

can we avoid having test depending on debug prints as much as we can? can check the final result only in this case.

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

nit: with the break at the end, it's probably more readable if we separate the continue by replacing the else if with if.

wlei added inline comments.Oct 13 2021, 4:37 PM
llvm/test/tools/llvm-profgen/inline-noprobe2.test
1–2

I see, the final result is enough for this case.

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

Good point!

wlei updated this revision to Diff 379553.Oct 13 2021, 4:38 PM

Updating D111749: [llvm-profgen] Also ignore the first external branch

wenlei accepted this revision.Oct 13 2021, 4:39 PM

lgtm, thanks.

wlei retitled this revision from [llvm-profgen] Also ignore the first external branch to [llvm-profgen] Ignore the whole trace with the leading external branch.Oct 13 2021, 4:50 PM
wlei edited the summary of this revision. (Show Details)
This revision was landed with ongoing or failed builds.Oct 13 2021, 5:04 PM
This revision was automatically updated to reflect the committed changes.