[PATCH] D17006: Refactor PGO function naming and MD5 hashing support out of ProfileData

[Xinliang David Li via llvm-commits]

On Mon, Feb 8, 2016 at 9:16 PM, Teresa Johnson <tejohnson at google.com> wrote:

>
>
> On Mon, Feb 8, 2016 at 9:04 PM, Xinliang David Li <davidxl at google.com>
> wrote:
>
>> On Mon, Feb 8, 2016 at 9:01 PM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>> > tejohnson added a comment.
>> >
>> > In http://reviews.llvm.org/D17006#347035, @davidxl wrote:
>> >
>> >> 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 think two interfaces are needed -- one takes Function*, while the
>> other takes StringRef. The later is for cases where Global string id
>> already known.
>>
>
> I'm still not clear on whether you want a new wrapper that does both the
> MD5Hash and the getGlobalIdentifier? I.e. for the former do you mean
> something like:
> uint64_t Function::getGlobalUID(Function *F) {
>    return MD5Hash(Function::getGlobalIdentifier(F->getName(),
> F->getLinkage(), F->getParent()->getName());
> }
>
> and for the latter do you mean something like:
> uint64_t Function::getGlobalUID(StringRef FuncName, LinkageType Linkage,
> StringRef FileName) {
>    return MD5Hash(Function::getGlobalIdentifier(FuncName, Linkage,
> FileName));
> }
>
> or just
>
> uint64_t Function::getGlobalUID(StringRef GlobalFuncName) {
>    return MD5Hash(GlobalFuncName);
> }
>

A simple wrapper like this one. Calling MD5Hash directly exposes too much
implementation details.



>
>
>
>
>> >I could add that with the follow-on patch to use this in the ThinLTO
>> function map.
>>
>> Ok -- It can be done a in separate patch. After that I will update PGO
>> to use them.
>>
>
> Initially at least I would only be able to use a StringRef interface, as I
> am pulling the linkage type out of the summary.
>

You mean the simple wrapper needed also by PGO?

David


>
>> >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.
>>
>> In most cases for PGO, the stringRef based interface will be used.
>>
>> thanks,
>>
>> David
>>
>> >
>> >
>> > http://reviews.llvm.org/D17006
>> >
>> >
>> >
>>
>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160208/24d5831f/attachment.html>