[JS Interop] Scan configureAll in the entire module, not just the start function

src/ir/intrinsics.cpp

@@ -16,6 +16,7 @@
 
 #include "ir/intrinsics.h"
 #include "ir/find_all.h"
+#include "ir/module-utils.h"
 #include "wasm-builder.h"
 
 namespace wasm {
@@ -102,27 +103,49 @@ std::vector<Name> Intrinsics::getConfigureAllFunctions(Call* call) {
 }
 
 std::vector<Name> Intrinsics::getJSCalledFunctions() {
-  std::vector<Name> ret;
+  using JSCalledSet = std::unordered_set<Name>;
+
+  // Gather the js.called functions, and find the configureAll, which can add
+  // more.
+  JSCalledSet jsCalled;
+  Function* configureAll = nullptr;
   for (auto& func : module.functions) {
     if (getAnnotations(func.get()).jsCalled) {
-      ret.push_back(func->name);
+      jsCalled.insert(func->name);
+    }
+
+    if (isConfigureAll(func.get())) {
+      configureAll = func.get();
     }
   }
 
-  // ConfigureAlls in a start function make their functions callable.
-  if (module.start) {
-    auto* start = module.getFunction(module.start);
-    if (!start->imported()) {
-      FindAll<Call> calls(start->body);
-      for (auto* call : calls.list) {
-        if (isConfigureAll(call)) {
-          for (auto name : getConfigureAllFunctions(call)) {
-            ret.push_back(name);
+  // ConfigureAlls make their functions callable. To avoid always scanning the
+  // the entire module, only do so when we saw the proper import.
+  if (configureAll) {
+    ModuleUtils::ParallelFunctionAnalysis<JSCalledSet> analysis(
+      module, [&](Function* func, JSCalledSet& jsCalled) {
+        if (func->imported()) {
+          return;
+        }
+
+        FindAll<Call> calls(func->body);
+        for (auto* call : calls.list) {
+          if (isConfigureAll(call)) {
+            for (auto name : getConfigureAllFunctions(call)) {
+              jsCalled.insert(name);
+            }
           }
         }
-      }
+      });
+
+    for (auto& [_, set] : analysis.map) {
+      jsCalled.insert(set.begin(), set.end());
     }
   }
+
+  // Return the set as a sorted vector to avoid nondeterminism.
+  std::vector<Name> ret(jsCalled.begin(), jsCalled.end());
+  std::sort(ret.begin(), ret.end());
   return ret;
 }
 

src/ir/module-utils.h

@@ -267,7 +267,8 @@ template<typename T> inline void iterModuleItems(Module& wasm, T visitor) {
 // The operation performed should not modify the wasm module in any way, by
 // default - otherwise, set the Mutability to Mutable. (This is not enforced at
 // compile time - TODO find a way - but at runtime in pass-debug mode it is
-// checked.)
+// checked. At compile time either const or non-const Module can be passed in,
+// depending on Mutability.)
 template<typename K, typename V> using DefaultMap = std::map<K, V>;
 template<typename T,
          Mutability Mut = Immutable,
@@ -280,7 +281,9 @@ struct ParallelFunctionAnalysis {
 
   using Func = std::function<void(Function*, T&)>;
 
-  ParallelFunctionAnalysis(Module& wasm, Func work) : wasm(wasm) {
+  ParallelFunctionAnalysis(
+    std::conditional_t<Mut == Immutable, const Module, Module>& wasm, Func work)
+    : wasm(const_cast<Module&>(wasm)) {
     // Fill in the map as we operate on it in parallel (each function to its own
     // entry).
     for (auto& func : wasm.functions) {

test/lit/passes/gufa-configureAll.wast

@@ -101,3 +101,99 @@
     )
   )
 )
+
+;; As above, but now the configureAll is *not* in a start function. We should
+;; still optimize in the same way as before. (In theory we could scan to see if
+;; the configureAll is actually reached, but we assume if it was not optimized
+;; out that it is.)
+(module
+  ;; CHECK:      (type $externs (array (mut externref)))
+  (type $externs (array (mut externref)))
+
+  ;; CHECK:      (type $funcs (array (mut funcref)))
+  (type $funcs (array (mut funcref)))
+
+  ;; CHECK:      (type $bytes (array (mut i8)))
+  (type $bytes (array (mut i8)))
+
+  ;; CHECK:      (type $3 (func (param i32) (result i32)))
+
+  ;; CHECK:      (type $configureAll (func (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
+  (type $configureAll (func (param (ref null $externs)) (param (ref null $funcs)) (param (ref null $bytes)) (param externref)))
+
+  ;; CHECK:      (type $5 (func))
+
+  ;; CHECK:      (import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll) (param (ref null $externs) (ref null $funcs) (ref null $bytes) externref)))
+  (import "wasm:js-prototypes" "configureAll" (func $configureAll (type $configureAll)))
+
+  ;; CHECK:      (data $bytes "12345678")
+  (data $bytes "12345678")
+
+  ;; CHECK:      (elem $externs externref (item (ref.null noextern)))
+  (elem $externs externref
+    (ref.null extern)
+  )
+
+  ;; CHECK:      (elem $funcs func $configured)
+  (elem $funcs funcref
+    (ref.func $configured)
+  )
+
+  ;; CHECK:      (elem $other func $unconfigured)
+  (elem $other funcref
+    (ref.func $unconfigured)
+  )
+
+  ;; CHECK:      (func $do-configure (type $5)
+  ;; CHECK-NEXT:  (call $configureAll
+  ;; CHECK-NEXT:   (array.new_elem $externs $externs
+  ;; CHECK-NEXT:    (i32.const 0)
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (array.new_elem $funcs $funcs
+  ;; CHECK-NEXT:    (i32.const 0)
+  ;; CHECK-NEXT:    (i32.const 1)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (array.new_data $bytes $bytes
+  ;; CHECK-NEXT:    (i32.const 0)
+  ;; CHECK-NEXT:    (i32.const 8)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:   (ref.null noextern)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $do-configure
+    (call $configureAll
+      (array.new_elem $externs $externs
+        (i32.const 0) (i32.const 1))
+      (array.new_elem $funcs $funcs
+        (i32.const 0) (i32.const 1))
+      (array.new_data $bytes $bytes
+        (i32.const 0) (i32.const 8))
+      (ref.null extern)
+    )
+  )
+
+  ;; CHECK:      (func $configured (type $3) (param $x i32) (result i32)
+  ;; CHECK-NEXT:  (i32.eqz
+  ;; CHECK-NEXT:   (local.get $x)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $configured (param $x i32) (result i32)
+    ;; This is not optimized out, because of the configureAll.
+    (i32.eqz
+      (local.get $x)
+    )
+  )
+
+  ;; CHECK:      (func $unconfigured (type $3) (param $x i32) (result i32)
+  ;; CHECK-NEXT:  (i32.eqz
+  ;; CHECK-NEXT:   (unreachable)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $unconfigured (param $x i32) (result i32)
+    ;; This is optimized out, because configureAll does not refer to it.
+    (i32.eqz
+      (local.get $x)
+    )
+  )
+)