This is an archive of the discontinued LLVM Phabricator instance.

Refactor PGO function naming and MD5 hashing support out of ProfileData
ClosedPublic

Authored by tejohnson on Feb 8 2016, 4:06 PM.

Details

Summary

Move the function renaming logic into the Function class, and the
MD5Hash routine into the MD5 header.

This will enable these routines to be shared with ThinLTO, which
will be changed to store the MD5 hash instead of full function name
in the combined index for significant size reductions. And using the same
function naming for locals in the function index facilitates future
integration with indirect call value profiles.

Diff Detail

Event Timeline

tejohnson updated this revision to Diff 47268.Feb 8 2016, 4:06 PM
tejohnson retitled this revision from to Refactor PGO function naming and MD5 hashing support out of ProfileData.
tejohnson updated this object.
tejohnson added a reviewer: davidxl.
tejohnson added a subscriber: llvm-commits.
davidxl added inline comments.Feb 8 2016, 4:16 PM
include/llvm/Support/MD5.h
70

Remove the 'static' keyword here (obviously it was there before but not cleaned up) -- to avoid leaving multiple copies of the function when inlining does not happen.

tejohnson updated this revision to Diff 47287.Feb 8 2016, 6:56 PM
  • Address David's comment.
davidxl accepted this revision.Feb 8 2016, 7:55 PM
davidxl edited edge metadata.

This one LGTM. I am also expecting another interface like 'getFunctionGuid' to wrap around the MD5Hash method..

This revision is now accepted and ready to land.Feb 8 2016, 7:55 PM

This one LGTM. I am also expecting another interface like 'getFunctionGuid' to wrap around the MD5Hash method..

Do you mean to basically do MD5Hash(getGlobalIdentifier(...))? I could add that with the follow-on patch to use this in the ThinLTO function map. Although for now in the draft patch I am testing they are invoked in slightly different places (the MD5Hash is hidden within the FunctionInfoIndex methods which take a string, and getGlobalIdentifier is invoked as needed above that. And for PGO, it didn't look like MD5Hash and getPGOFuncName were invoked simultaneously.

This revision was automatically updated to reflect the committed changes.