This is an archive of the discontinued LLVM Phabricator instance.

[profile] Move __llvm_profile_filename into a separate object
ClosedPublic

Authored by vsk on Jun 28 2017, 6:26 PM.

Details

Summary

Users can specify the path a raw profile is written to by passing
-fprofile-instr-generate=<path>, but this functionality broke on Darwin
after __llvm_profile_filename was made weak [1], resulting in profiles
being written to "default.profraw" even when <path> is specified.

The situation is that instrumented programs provide a weak definition of
__llvm_profile_filename, which conflicts with a weak redefinition
provided by the profiling runtime.

The linker appears to pick the 'winning' definition arbitrarily: on
Darwin, it usually prefers the larger definition, which is probably why
the instrprof-override-filename.c test has been passing.

The fix is to move the runtime's definition into a separate object file
within the archive. This means that the linker won't "see" the runtime's
definition unless the user program has not provided one. I couldn't
think of a great way to test this other than to mimic the Darwin
failure: use -fprofile-instr-generate=<some-small-path>.

Testing: check-{clang,profile}, modified instrprof-override-filename.c.

[1] [Profile] deprecate __llvm_profile_override_default_filename
https://reviews.llvm.org/D22613
https://reviews.llvm.org/D22614

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Jun 28 2017, 6:26 PM
davidxl added inline comments.Jun 28 2017, 10:03 PM
test/profile/instrprof-override-filename.c
6 ↗(On Diff #104575)

can you add a test for IR PGO too using %clang_pgogen=%t.dir/ ...

arphaman edited edge metadata.Jun 29 2017, 9:59 AM

lg from my side, thanks

vsk updated this revision to Diff 104678.Jun 29 2017, 10:31 AM

Address David's review comment. I could not use the exact same test structure for IR PGO due to what seems like a spurious hash mismatch (see the FIXME).

vsk updated this revision to Diff 104682.Jun 29 2017, 10:39 AM

Per David's suggestion, make sure the pipelines for the clang_pgogen/clang_profuse pair match.

This revision is now accepted and ready to land.Jun 29 2017, 10:41 AM
This revision was automatically updated to reflect the committed changes.