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

[Teresa Johnson via llvm-commits]

On Mon, Feb 8, 2016 at 9:25 PM, Xinliang David Li <davidxl at google.com>
wrote:

>
>
> 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.
>

Ok, got it. That's easy and I can use it in my follow-on patch.

>
>
>
>>
>>
>>
>>
>>> >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?
>

Sorry, I don't follow the question?



> 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
>>
>
>


-- 
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/20160209/1de74a52/attachment.html>