[PATCH] D108342: [CSSPGO] Enable loading MD5 CS profile.

Hongtao Yu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 30 22:10:33 PDT 2021


hoy added a comment.

In D108342#2973938 <https://reviews.llvm.org/D108342#2973938>, @wenlei wrote:

> In D108342#2973929 <https://reviews.llvm.org/D108342#2973929>, @hoy wrote:
>
>> In D108342#2973777 <https://reviews.llvm.org/D108342#2973777>, @wenlei wrote:
>>
>>>> I had to use llvm-profdata simply because of the support of md5 in llvm-profgen was not ready. It shouldn't be too slow now for llvm-profgen to use real names with the context split work. We could give Lei's approach a shot to see how much win can be get.
>>>
>>> What I was wondering is what does it take for md5 support to be ready for llvm-profgen? I thought it's close to a no-op as llvm-profgen use the same writer, and the handling md5 is in the writer. Of course we can use Lei's change to make it faster, but that's just optimization.
>>
>> I see. It should be straightforward to add md5 support to llvm-profgen.  I just added here.
>
> Ok, thanks. Yeah this is what I'm trying to understand - why not adding it here since it should be one-liner. :)
>
> Can you add md5 test case for llvm-profgen too? (We also need to address preinliner with md5 asap, ok for a later patch)

A test is already added to noinline-cs-noprobe.test. Do you mean in another form? Since md5 profile is not text, I'm not sure what's a good way to check its context.



================
Comment at: llvm/tools/llvm-profgen/ProfileGenerator.cpp:32
+    "use-md5", cl::Hidden, cl::init(false),
+    cl::desc("Use md5 to represent function names in the output profile"));
+
----------------
wenlei wrote:
> Similar to llvm-profdata, call out in description that this is only meaningful for -extbinary.
Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108342/new/

https://reviews.llvm.org/D108342



More information about the llvm-commits mailing list