[PATCH] D51643: [SampleFDO] Make sample profile loader unaware of compact format change

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 6 10:41:53 PDT 2018


On Thu, Sep 6, 2018 at 10:30 AM, Dehao Chen via Phabricator
<reviews at reviews.llvm.org> wrote:
> danielcdh added inline comments.
>
>
> ================
> Comment at: include/llvm/ProfileData/SampleProf.h:464
> +        /// original local name. In sample profile, the suffixes of function
> +        /// names are all stripped. Since it is possible that the mapper is
> +        /// built in post-thin-link phase and var promotion has been done,
> ----------------
> wmi wrote:
>> davidxl wrote:
>> > why are those suffixes stripped in sample profile?
>> It is possible that the binary sampled is built in thinlto mode and some functions are promoted. The profile generated from such binary will contain names after promotion. If such profile is used in thinlto pre-link-phase or nonthinlto build, we need to strip those names from sample before we can find a match in the module.
>>
>> Indeed if there are multiple local functions with the same name are promoted, their samples will be merged together after the strip, which will introduce some imprecision. Maybe this part can be improved -- don't strip names from sample, but always promote the local names in the module before trying to match it with the name from sample. I am not sure whether the effort is worthy. Dehao may already evaluate it before?
> I remember that name stripping is important as there are many different types of suffixes added to the function name which makes it impossible to match. But I do not remember the exact impact. It may be worth re-evaluate.
>
> For the local functions, what's the promoted name look like after thinlto? If it's deterministic, then we should not strip it.

It is origname + ".llvm." + low32bits_module_md5. There will be
problem if the module changes a little.

Wei.

>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D51643
>
>
>


More information about the llvm-commits mailing list