forked from OSchip/llvm-project
[ThinLTO] Refine reachability check to fix compile time increase
Summary: A recent fix to the ThinLTO whole program dead code elimination (D56117) increased the thin link time on a large MSAN'ed binary by 2x. It's likely that the time increased elsewhere, but was more noticeable here since it was already large and ended up timing out. That change made it so we would repeatedly scan all copies of linkonce symbols for liveness every time they were encountered during the graph traversal. This was needed since we only mark one copy of an aliasee as live when we encounter a live alias. This patch fixes the issue in a more efficient manner by simply proactively visiting the aliasee (thus marking all copies live) when we encounter a live alias. Two notes: One, this requires a hash table lookup (finding the aliasee summary in the index based on aliasee GUID). However, the impact of this seems to be small compared to the original pre-D56117 thin link time. It could be addressed if we keep the aliasee ValueInfo in the alias summary instead of the aliasee GUID, which I am exploring in a separate patch. Second, we only populate the aliasee GUID field when reading summaries from bitcode (whether we are reading individual summaries and merging on the fly to form the compiled index, or reading in a serialized combined index). Thankfully, that's currently the only way we can get to this code as we don't yet support reading summaries from LLVM assembly directly into a tool that performs the thin link (they must be converted to bitcode first). I added a FIXME, however I have the fix under test already. The easiest fix is to simply populate this field always, which isn't hard, but more likely the change I am exploring to store the ValueInfo instead as described above will subsume this. I don't want to hold up the regression fix for this though. Reviewers: trentxintong Subscribers: mehdi_amini, inglorion, dexonsmith, llvm-commits Differential Revision: https://reviews.llvm.org/D57203 llvm-svn: 352438
This commit is contained in:
parent
a36a293a56
commit
5b2f6a1bc2
|
@ -777,9 +777,7 @@ void llvm::computeDeadSymbols(
|
|||
if (!VI)
|
||||
return;
|
||||
|
||||
// We need to make sure all variants of the symbol are scanned, alias can
|
||||
// make one (but not all) alive.
|
||||
if (llvm::all_of(VI.getSummaryList(),
|
||||
if (llvm::any_of(VI.getSummaryList(),
|
||||
[](const std::unique_ptr<llvm::GlobalValueSummary> &S) {
|
||||
return S->isLive();
|
||||
}))
|
||||
|
@ -819,12 +817,23 @@ void llvm::computeDeadSymbols(
|
|||
while (!Worklist.empty()) {
|
||||
auto VI = Worklist.pop_back_val();
|
||||
for (auto &Summary : VI.getSummaryList()) {
|
||||
GlobalValueSummary *Base = Summary->getBaseObject();
|
||||
// Set base value live in case it is an alias.
|
||||
Base->setLive(true);
|
||||
for (auto Ref : Base->refs())
|
||||
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
|
||||
// processing of its references.
|
||||
// FIXME: The aliasee GUID is only populated in the summary when we
|
||||
// read them from bitcode, which is currently the only way we can
|
||||
// get here (we don't yet support reading the summary index directly
|
||||
// from LLVM assembly code in tools that can perform a thin link).
|
||||
// If that ever changes, the below call to getAliaseGUID will assert.
|
||||
visit(Index.getValueInfo(AS->getAliaseeGUID()));
|
||||
continue;
|
||||
}
|
||||
|
||||
Summary->setLive(true);
|
||||
for (auto Ref : Summary->refs())
|
||||
visit(Ref);
|
||||
if (auto *FS = dyn_cast<FunctionSummary>(Base))
|
||||
if (auto *FS = dyn_cast<FunctionSummary>(Summary.get()))
|
||||
for (auto Call : FS->calls())
|
||||
visit(Call.first);
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue