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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
