Page MenuHomePhabricator

[PGO] Add a functionality to always instrument the func entry BB
ClosedPublic

Authored by xur on Jun 18 2020, 1:22 PM.

Details

Summary

Add an option to always instrument function entry BB (default off)
Add an options to do atomic update the first counter in each instrumented function.

Diff Detail

Event Timeline

xur created this revision.Jun 18 2020, 1:22 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 18 2020, 1:22 PM
Herald added subscribers: jfb, hiraditya. · View Herald Transcript
davidxl added inline comments.Jun 18 2020, 2:15 PM
llvm/lib/Transforms/Instrumentation/CFGMST.h
110

Does this guarantee entry be instrumented?

147

Is this the way to make sure entry is always instrumented?

292

is this needed? Can somthing be done in CompputeMST without relying on weight trick?

ok. Why atomic update just for first entry? are they particularly more contended than other counters?

wmi added a comment.Jun 18 2020, 3:20 PM

Thanks for the patch! I guess atomic update is to reduce the cost of the entry counter for application with many threads. How much difference there is w/wo enabling -atomic-first-counter?

wmi added a comment.Jun 21 2020, 10:34 PM

I tried it on an internal benchmark.

base: head of trunk
exp: pgo-instrument-entry=true, atomic-first-counter=false
qps of Instrumented binary run: 344.63 vs 340.56
optimized binary run: qps had no significant change in 6 runs. latency had ~0.1% regression in 3 runs and no regression in other 3 runs.

base: head of trunk
exp: pgo-instrument-entry=true, atomic-first-counter=true
qps of Instrumented binary run: 342.53 vs 265.58
optimized binary run: qps had no significant change in 2 runs, and had ~0.11% improvement in 4 runs. latency had ~0.35% improvement in 6 runs.

The test showed always inserting counter in the entry block had minor impact on instrumented binary and optimized binary runs. Using atomic entry counter could improve profile quality but significantly slowdown instrument binary.

davidxl accepted this revision.Jun 25 2020, 9:23 AM

LGTM. Versioning support is needed when it is flipped on by default.

This revision is now accepted and ready to land.Jun 25 2020, 9:23 AM
This revision was automatically updated to reflect the committed changes.