[PATCH] D45531: [LTO] Add stats-file option to LTO/Config.h.

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson accepted this revision.
tejohnson added a comment.
This revision is now accepted and ready to land.

LGTM but please wait until after https://reviews.llvm.org/D45820 goes in.



================
Comment at: tools/gold/gold-plugin.cpp:1057
     if (llvm::AreStatisticsEnabled())
       llvm::PrintStatistics();
     cleanup_hook();
----------------
tejohnson wrote:
> I think there is going to be a strange interaction with this code. I added this so that we still get the stats printed to stderr under the -stats llvm internal option when we exit early because we are only doing the ThinLTO indexing step (for distrubuted ThinLTO builds), since we don't execute ~StatisticInfo() in that case. The problem is that AreStatisticsEnabled() returns "Enabled || Stats", and Enabled will be true with this patch if the new stats-file plugin option is specified. So I think what would happen for distributed builds, when we exit early here, if the new plugin option is passed we will incorrectly dump stats to stderr as well.
> 
> Hmm, looks like we should probably just call llvm_shutdown() here before we exit, which would take care of destructing StatisticInfo. Let me try that and get back...
Just mailed you D45820 that fixes this.


https://reviews.llvm.org/D45531