maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

[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
This commit is contained in:
Teresa Johnson 2020-07-30 13:49:49 -07:00
parent c068e9c8c1
commit 1479cdfe4f
2 changed files with 25 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
llvm/lib/IR/ModuleSummaryIndex.cpp
View File

@ -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())

3
llvm/lib/Transforms/IPO/FunctionImport.cpp
View File

@ -884,6 +884,7 @@ void llvm::computeDeadSymbols(
while (!Worklist.empty()) {
auto VI = Worklist.pop_back_val();
for (auto &Summary : VI.getSummaryList()) {
Summary->setLive(true);
if (auto *AS = dyn_cast<AliasSummary>(Summary.get())) {
// If this is an alias, visit the aliasee VI to ensure that all copies
// are marked live and it is added to the worklist for further
@ -891,8 +892,6 @@ void llvm::computeDeadSymbols(
visit(AS->getAliaseeVI(), true);
continue;
}
Summary->setLive(true);
for (auto Ref : Summary->refs())
visit(Ref, false);
if (auto *FS = dyn_cast<FunctionSummary>(Summary.get()))