From 1479cdfe4ff603e7b0140dab3ca08ff095473cbd Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 30 Jul 2020 13:49:49 -0700 Subject: [ThinLTO] Compile time improvement to propagateAttributes I found that propagateAttributes was ~23% of a thin link's run time (almost 4x higher than the second hottest function). The main reason is that it re-examines a global var each time it is referenced. This becomes unnecessary once it is marked both non read only and non write only. I added a set to avoid doing redundant work, which dropped the runtime of that thin link by almost 15%. I made a smaller efficiency improvement (no measurable impact) to skip all summaries for a VI if the first copy is dead. I added an assert to ensure that all copies are dead if any is. The code in computeDeadSymbols marks all summaries for a VI as live. There is one corner case where it was skipping marking an alias as live, that I fixed. However, since the code earlier marked all copies of a preserved GUID's VI as live, and each 'visit' marks all copies live, the only case where this could make a difference is summaries that were marked live when they were built initially, and that is only a few special compiler generated symbols and inline assembly symbols, so it likely is never provoked in practice. Differential Revision: https://reviews.llvm.org/D84985 --- llvm/lib/IR/ModuleSummaryIndex.cpp | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) (limited to 'llvm/lib/IR/ModuleSummaryIndex.cpp') diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp index 91612ea..5346323 100644 --- a/llvm/lib/IR/ModuleSummaryIndex.cpp +++ b/llvm/lib/IR/ModuleSummaryIndex.cpp @@ -163,7 +163,9 @@ bool ModuleSummaryIndex::isGUIDLive(GlobalValue::GUID GUID) const { return false; } -static void propagateAttributesToRefs(GlobalValueSummary *S) { +static void +propagateAttributesToRefs(GlobalValueSummary *S, + DenseSet &MarkedNonReadWriteOnly) { // If reference is not readonly or writeonly then referenced summary is not // read/writeonly either. Note that: // - All references from GlobalVarSummary are conservatively considered as @@ -174,6 +176,11 @@ static void propagateAttributesToRefs(GlobalValueSummary *S) { // for them. for (auto &VI : S->refs()) { assert(VI.getAccessSpecifier() == 0 || isa(S)); + if (!VI.getAccessSpecifier()) { + if (!MarkedNonReadWriteOnly.insert(VI).second) + continue; + } else if (MarkedNonReadWriteOnly.find(VI) != MarkedNonReadWriteOnly.end()) + continue; for (auto &Ref : VI.getSummaryList()) // If references to alias is not read/writeonly then aliasee // is not read/writeonly @@ -216,11 +223,24 @@ void ModuleSummaryIndex::propagateAttributes( const DenseSet &GUIDPreservedSymbols) { if (!PropagateAttrs) return; + DenseSet MarkedNonReadWriteOnly; for (auto &P : *this) for (auto &S : P.second.SummaryList) { - if (!isGlobalValueLive(S.get())) + if (!isGlobalValueLive(S.get())) { + // computeDeadSymbols should have marked all copies live. Note that + // it is possible that there is a GUID collision between internal + // symbols with the same name in different files of the same name but + // not enough distinguishing path. Because computeDeadSymbols should + // conservatively mark all copies live we can assert here that all are + // dead if any copy is dead. + assert(llvm::none_of( + P.second.SummaryList, + [&](const std::unique_ptr &Summary) { + return isGlobalValueLive(Summary.get()); + })); // We don't examine references from dead objects - continue; + break; + } // Global variable can't be marked read/writeonly if it is not eligible // to import since we need to ensure that all external references get @@ -240,7 +260,7 @@ void ModuleSummaryIndex::propagateAttributes( GVS->setReadOnly(false); GVS->setWriteOnly(false); } - propagateAttributesToRefs(S.get()); + propagateAttributesToRefs(S.get(), MarkedNonReadWriteOnly); } setWithAttributePropagation(); if (llvm::AreStatisticsEnabled()) -- cgit v1.1