yonghong-song wrote: > I did some experiments for ~30 of our large targets. I set > -always-rename-promoted-locals to true so that I didn't get linking errors > (since we use distributed ThinLTO). I added a separate option to LTO.cpp that > when true prevented initializing ExternallyVisibleSymbolNamesPtr when enabled > (so that we don't go through any of the analysis in the thin link). I ran the > thin link 3x with that new option true and then false and compared the > results. > > I am seeing a fairly persistent increase in time by 2.5% on average (range is > neutral to ~6%) with most increasing 1-3%. Given that the thin link is > serial, and for our large targets ever bit helps, I think this should be off > by default (enable always rename) at least initially, as it isn't enabling a > performance improvement but rather more to simplify the kernel patching. > > To do this, I'd suggest making the AlwaysRenamePromotedLocals option a global > in the llvm namespace instead of file static, and using that to prevent the > analysis in LTO.cpp, like the following: > > ``` > DenseSet<StringRef> *ExternallyVisibleSymbolNamesPtr = > (WholeProgramVisibilityEnabledInLTO && !AlwaysRenamePromotedLocals) > ? &ExternallyVisibleSymbolNames > : nullptr; > ``` > I changed AlwaysRenamePromotedLocals to DisableAlwaysRenamePromotedLocals (false by default). So the above !AlwaysRenamePromotedLocals becomes DisableAlwaysRenamePromotedLocals.
> This can then also suffice to keep it off for distributed ThinLTO for now > (add a FIXME where the option is defined in FunctionImportUtils.cpp that it > needs to be off by default for that reason). I added a FIXME comment in option DisableAlwaysRenamePromotedLocals to explain that off is off by default due to distributed thinlto. > > I have a few other minor comments for the patch, let me go through it now and > send those, so that with the above change you can wrap this up and commit it. I fixed all these minor changes. Please see the latest commit for difference. https://github.com/llvm/llvm-project/pull/178587 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
