DeadArgumentElimination: Skip unprofitable single-call chains (#8072)

If a codebase has a long chain of single calls

a -> b -> c -> d

then we can end up in a very slow and unprofitable situation, removing
params from c's call to d, which then means c does not use some params,
and then we go back and so forth. Each step back requires a full scan of
the code. We could develop a more sophisticated IR to handle this, but it
would need to track that the local.get of the incoming param is only used
in an outgoing param, etc., which is not trivial, and these chains are
unprofitable in another way: single calls like this are inlined anyhow,
making this work redundant.

For simplicity, this PR detects the most obvious case of such a chain -
that an iteratiion of work only found a single single-caller call to remove
params from - and stops removing params from that point.

This makes this pass 30% faster on a large Dart testcase, which makes
-O3 9% faster. On other large wasm files I see much smaller benefits,
but it helps sometimes there too.

This is not truly NFC as while inlining will handle this case after, we do
alter the order a bit, leading to slightly different outputs sometimes. (The
changes are not better or worse, just noise.)

Author: kripken

src/passes/DeadArgumentElimination.cpp

@@ -233,6 +233,23 @@ struct DAE : public Pass {
   // and then use that possibly-over-approximating data.
   std::vector<std::vector<Name>> 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