<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 8, 2016 at 9:16 PM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Mon, Feb 8, 2016 at 9:04 PM, Xinliang David Li <span dir="ltr"><<a href="mailto:davidxl@google.com" target="_blank">davidxl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span>On Mon, Feb 8, 2016 at 9:01 PM, Teresa Johnson <<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a>> wrote:<br>
> tejohnson added a comment.<br>
><br>
> In <a href="http://reviews.llvm.org/D17006#347035" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17006#347035</a>, @davidxl wrote:<br>
><br>
>> This one LGTM. I am also expecting another interface like  'getFunctionGuid' to wrap around the MD5Hash method..<br>
><br>
><br>
> Do you mean to basically do MD5Hash(getGlobalIdentifier(...))?<br>
<br>
</span>I think two interfaces are needed -- one takes Function*, while the<br>
other takes StringRef. The later is for cases where Global string id<br>
already known.<br></blockquote><div><br></div></span><div>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:</div><div>uint64_t Function::getGlobalUID(Function *F) {<br></div><div><div>   return MD5Hash(Function::getGlobalIdentifier(F->getName(), F->getLinkage(), F->getParent()->getName());</div></div><div>}</div><div><br></div><div>and for the latter do you mean something like:</div><div><div>uint64_t Function::getGlobalUID(StringRef FuncName, LinkageType Linkage, StringRef FileName) {<br></div><div>   return MD5Hash(Function::getGlobalIdentifier(FuncName, Linkage, FileName));</div><div>}</div></div><div><br></div><div>or just</div><div><br></div><div><div>uint64_t Function::getGlobalUID(StringRef GlobalFuncName) {<br></div><div>   return MD5Hash(GlobalFuncName);</div><div>}</div></div></div></div></div></blockquote><div><br></div><div>A simple wrapper like this one. Calling MD5Hash directly exposes too much implementation details.</div><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><div><br></div><div><br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span><br>
>I could add that with the follow-on patch to use this in the ThinLTO function map.<br>
<br>
</span>Ok -- It can be done a in separate patch. After that I will update PGO<br>
to use them.<br></blockquote><div><br></div></span><div>Initially at least I would only be able to use a StringRef interface, as I am pulling the linkage type out of the summary.  </div></div></div></div></blockquote><div><br></div><div>You mean the simple wrapper needed also by PGO?</div><div><br></div><div>David</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<span><br>
>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.<br>
<br>
</span>In most cases for PGO, the stringRef based interface will be used.<br>
<br>
thanks,<br>
<br>
David<br>
<br>
><br>
><br>
> <a href="http://reviews.llvm.org/D17006" rel="noreferrer" target="_blank">http://reviews.llvm.org/D17006</a><br>
><br>
><br>
><br>
</blockquote></span></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><div><span style="font-family:Times;font-size:medium"><table cellspacing="0" cellpadding="0"><tbody><tr style="color:rgb(85,85,85);font-family:sans-serif;font-size:small"><td nowrap style="border-top-style:solid;border-top-color:rgb(213,15,37);border-top-width:2px">Teresa Johnson |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(51,105,232);border-top-width:2px"> Software Engineer |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(0,153,57);border-top-width:2px"> <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> |</td><td nowrap style="border-top-style:solid;border-top-color:rgb(238,178,17);border-top-width:2px"> <a href="tel:408-460-2413" value="+14084602413" target="_blank">408-460-2413</a></td></tr></tbody></table></span></div>
</font></span></div></div>
</blockquote></div><br></div></div>