This is an archive of the discontinued LLVM Phabricator instance.

[PGO] Add API to merge profile from buffer
ClosedPublic

Authored by davidxl on Mar 2 2016, 3:07 PM.

Details

Summary

Notes: since VP is not supported with buffer API, the test case for VP merging is not added. The API to do profile/in-process counter matching check will be in a followup patch.

Diff Detail

Repository
rL LLVM

Event Timeline

davidxl updated this revision to Diff 49677.Mar 2 2016, 3:07 PM
davidxl retitled this revision from to [PGO] Add API to merge profile from buffer.
davidxl updated this object.
davidxl added a reviewer: vsk.
davidxl added subscribers: llvm-commits, bogner, silvas.
vsk edited edge metadata.Mar 2 2016, 3:24 PM

Lgtm with minor changes.

lib/profile/InstrProfiling.h
66 ↗(On Diff #49677)

nit, from

lib/profile/InstrProfilingMerge.c
27 ↗(On Diff #49677)

It's documented that the source/destination profile buffers should have the same layout, but we should return early if data_size(data_begin(), data_end()) != Header->DataSize. The same goes for CountersSize and NamesSize.

60 ↗(On Diff #49677)

Please split this loop nest into a separate function.

make/platform/clang_linux.mk
83 ↗(On Diff #49677)

nit, indentation

silvas added a comment.Mar 2 2016, 3:39 PM

Hi David, this looks really nice. You were right that the profraw merging is quite simple.

Can you update instrprof-without-libc.c test to include __llvm_profile_merge_from_buffer?

Also, would it be reasonable to omit the value prof merging until we can test it? If not, that's fine, but it may simplify this patch.

test/profile/instrprof-merge.c
94 ↗(On Diff #49677)

Nit: Note -> Not

davidxl updated this revision to Diff 49758.Mar 3 2016, 10:58 AM
davidxl updated this object.
davidxl edited edge metadata.

Addressed all review feedbacks from vsk and sean:

  1. split the vp merger to avoid introduce libc dependency on buffer API client
  2. added a test in instrprof-without-lib.c by calling buffer merge API
  3. added a test case for VP data merging
This revision was automatically updated to reflect the committed changes.