diff options
author | Teresa Johnson <tejohnson@google.com> | 2020-07-30 13:49:49 -0700 |
---|---|---|
committer | Teresa Johnson <tejohnson@google.com> | 2020-07-31 10:54:02 -0700 |
commit | 1479cdfe4ff603e7b0140dab3ca08ff095473cbd (patch) | |
tree | be5cedf71ae53052a1d156d70fda3c10d25d8613 /llvm/lib/IR/ModuleSummaryIndex.cpp | |
parent | c068e9c8c123e7f8c8f3feb57245a012ccd09ccf (diff) | |
download | llvm-1479cdfe4ff603e7b0140dab3ca08ff095473cbd.zip llvm-1479cdfe4ff603e7b0140dab3ca08ff095473cbd.tar.gz llvm-1479cdfe4ff603e7b0140dab3ca08ff095473cbd.tar.bz2 |
[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
Diffstat (limited to 'llvm/lib/IR/ModuleSummaryIndex.cpp')
-rw-r--r-- | llvm/lib/IR/ModuleSummaryIndex.cpp | 28 |
1 files changed, 24 insertions, 4 deletions
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<ValueInfo> &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<FunctionSummary>(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<GlobalValue::GUID> &GUIDPreservedSymbols) { if (!PropagateAttrs) return; + DenseSet<ValueInfo> 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<GlobalValueSummary> &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()) |