This is an archive of the discontinued LLVM Phabricator instance.

[Profile] compiler-rt support for setting profile output from command line
ClosedPublic

Authored by tejohnson on Apr 28 2015, 7:54 AM.

Details

Reviewers
tejohnson
bogner
Summary

This change is the first of 3 patches to add support for specifying
the profile output from the command line via -fprofile-instr-generate=<path>,
where the specified output path/file will be overridden by the
LLVM_PROFILE_FILE environment variable.

Several changes are made to the runtime to support this:

  1. Add a new interface __llvm_profile_set_filename_env_override that will

set the profile output filename, but allows the LLVM_PROFILE_FILE to override.
This is the interface used by the new option.

  1. Refactor the pid-expansion done for LLVM_PROFILE_FILE into a separate

routine that can be shared by the various filename setting routines
(so that the filename from the option can also use the "%p" syntax).

  1. Move the truncation into setFilename, and only truncate if there is a

new filename specified (to maintain support for appending to the same
profile file in the case of multiple shared objects built with profiling).

  1. Move the handling for a NULL filename passed to __llvm_profile_setfilename*

into the new setFilenamePossiblyWithPid routine. The handling for a null
LLVM_PROFILE_FILE (which should not reset) is done by caller
setFilenameFromEnvironment.

Diff Detail

Event Timeline

tejohnson updated this revision to Diff 24550.Apr 28 2015, 7:54 AM
tejohnson retitled this revision from to [Profile] compiler-rt support for setting profile output from command line.
tejohnson updated this object.
tejohnson edited the test plan for this revision. (Show Details)
tejohnson added a subscriber: Unknown Object (MLST).
davidxl added inline comments.
lib/profile/InstrProfiling.h
92

Is it better to merge this interface with the previous one but with an additional parameter ?

lib/profile/InstrProfilingFile.c
187–188

The behavior of this interface changes (improves) slightly. Is this intended?

tejohnson added inline comments.Apr 28 2015, 9:44 AM
lib/profile/InstrProfiling.h
92

I didn't want to affect any existing users of the __llvm_profile_set_filename interface by adding a new parameter (since this is C not C++ and can't add parameter defaults).

lib/profile/InstrProfilingFile.c
187–188

Yes. I am guessing that the lack of %p handling in the existing __llvm_profile_set_filename was probably just because no one needed it.

tejohnson updated this revision to Diff 24575.Apr 28 2015, 1:34 PM

Re: [Profile] compiler-rt support for setting profile output from command line

Updated patch based on comments from Justin:

  • Passing NULL to the filename setting interfaces now resets to default.profraw as

indicated in the header file description.

  • Changed a couple of routine names to be clearer.
  • New tests.

Adding a link to Justin's LGTM since it didn't show up here:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150427/273749.html

Committed as r236055 and r236056.

tejohnson accepted this revision.Apr 28 2015, 4:35 PM
tejohnson added a reviewer: tejohnson.

Marking accepted since it was LGTM'ed and committed.

This revision is now accepted and ready to land.Apr 28 2015, 4:35 PM
tejohnson closed this revision.Aug 12 2015, 7:47 AM

Committed awhile back as r236055 and r236056, closing manually.