diff --git a/src/passes/DeadArgumentElimination.cpp b/src/passes/DeadArgumentElimination.cpp index 42d8d87a6a9..27cc915e0a4 100644 --- a/src/passes/DeadArgumentElimination.cpp +++ b/src/passes/DeadArgumentElimination.cpp @@ -233,6 +233,23 @@ struct DAE : public Pass { // and then use that possibly-over-approximating data. std::vector> callers; + // A count of how many iterations we saw unprofitable removals of parameters. + // An unprofitable removal is one where we only manage to remove from a single + // call, that is, from one call target and it has a single call going to it. + // Such calls are not very interesting, as when there is a single call like + // that then inlining will handle it anyhow, in most cases, and inlining + // does so far more efficiently in situations of call chains: + // + // a -> b -> c -> d + // + // Imagine we remove a param from d, and so we remove it from the call in c. + // If c received that as a parameter, and only ever used it to call d, then + // now we can remove a param from c, and from the call in b, and so forth - + // requiring a full iteration each time to find the small amount of progress. + // (Inlining, otoh, will inline b into a, then c into a, and d into a, + // efficiently.) + Index unprofitableRemovalIters = 0; + bool iteration(Module* module, DAEFunctionInfoMap& infoMap) { allDroppedCalls.clear(); @@ -381,33 +398,58 @@ struct DAE : public Pass { ReFinalize().run(getPassRunner(), module); } // We now know which parameters are unused, and can potentially remove them. - for (Index index = 0; index < numFunctions; index++) { - auto* func = module->functions[index].get(); - if (func->imported()) { - continue; - } - if (hasUnseenCalls[index]) { - continue; - } - auto numParams = func->getNumParams(); - if (numParams == 0) { - continue; - } - auto& calls = allCalls[index]; - if (calls.empty()) { - continue; - } - auto name = func->name; - auto [removedIndexes, outcome] = ParamUtils::removeParameters( - {func}, infoMap[name].unusedParams, calls, {}, module, getPassRunner()); - if (!removedIndexes.empty()) { - // Success! - worthOptimizing.insert(func); - markStale(name); - markCallersStale(index); + // Only do so if we didn't run into unprofitable removals - if so, leave + // any further removals for other invocations of this pass. (This avoids us + // getting stuck in long unprofitable call chains as mentioned in the + // comment earlier; note that we do process one unprofitable iteration + // before giving up here, so we do make progress at least.) + if (!unprofitableRemovalIters) { + Index removals = 0; + Index singleCallerRemovals = 0; + for (Index index = 0; index < numFunctions; index++) { + auto* func = module->functions[index].get(); + if (func->imported()) { + continue; + } + if (hasUnseenCalls[index]) { + continue; + } + auto numParams = func->getNumParams(); + if (numParams == 0) { + continue; + } + auto& calls = allCalls[index]; + if (calls.empty()) { + continue; + } + auto name = func->name; + auto [removedIndexes, outcome] = + ParamUtils::removeParameters({func}, + infoMap[name].unusedParams, + calls, + {}, + module, + getPassRunner()); + if (!removedIndexes.empty()) { + // Success! + worthOptimizing.insert(func); + markStale(name); + markCallersStale(index); + if (calls.size() == 1) { + singleCallerRemovals++; + } + removals++; + } + if (outcome == ParamUtils::RemovalOutcome::Failure) { + callTargetsToLocalize.insert(name); + } } - if (outcome == ParamUtils::RemovalOutcome::Failure) { - callTargetsToLocalize.insert(name); + if (removals == 1 && singleCallerRemovals == 1 && + callTargetsToLocalize.empty()) { + // We only removed parameters from one function, and it had a single + // caller, and we don't have other pending actions (call targets we + // need to localize), so this was unprofitable as mentioned earlier. + unprofitableRemovalIters++; } } // We can also tell which calls have all their return values dropped. Note