[PATCH] D73307: Unique Names for Functions with Internal Linkage

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D73307#1839880 <https://reviews.llvm.org/D73307#1839880>, @mtrofin wrote:

> In D73307#1839829 <https://reviews.llvm.org/D73307#1839829>, @MaskRay wrote:
>
> > The code change seems fine, but the test requires some changes. I haven't followed Propeller development, but hope someone with profile experience can confirm InternalLinkage is the only linkage we need to care about (otherwise the option will be a misnomer if we ever extend it) and check whether this feature is useful on its own. Does it improve profile precision?
>
>
> I can comment on the usefulness aspect: we had an earlier prototype of this, which we tried on a real-world application benchmark. The binary had ~10% of local statics exhibiting duplicate names. Ensuring unique names led to observable differences in the AFDO file (i.e. some of those functions had profiles that, before, were lost for one of the duplicates, and now were correctly attributed to the different functions), and a measurable performance improvement.


Thanks! Do you happen to have numbers about the code size increase? Every InternalLinkage function will have a longer time. They may take a significant portion of the string table (.strtab) size. If you strip .strtab, the profiling precision will be lowered to the situation before.


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

https://reviews.llvm.org/D73307