[GUFA] Handle GUFA + Intrinsics (#4839)

Like RemoveUnusedModuleElements, places that build graphs of function
reachability must special-case the call-without-effects intrinsic. Without that,
it looks like a call to an import. Normally a call to an import is fine - it makes us
be super-pessimistic, as we think things escape all the way out - but in GC
for now we are assuming a closed world, and so we end up broken. To fix that,
properly handle the intrinsic case.

2 files changed, 52 insertions, 17 deletions

diff --git a/src/ir/possible-contents.cpp b/src/ir/possible-contents.cpp
index 3677516b3..67891f45f 100644
--- a/src/ir/possible-contents.cpp
+++ b/src/ir/possible-contents.cpp
@@ -502,8 +502,9 @@ struct InfoCollector
 
   // Calls send values to params in their possible targets, and receive
   // results.
-  void visitCall(Call* curr) {
-    auto* target = getModule()->getFunction(curr->target);
+
+  template<typename T> void handleDirectCall(T* curr, Name targetName) {
+    auto* target = getModule()->getFunction(targetName);
     handleCall(
       curr,
       [&](Index i) {
@@ -513,9 +514,7 @@ struct InfoCollector
         return ResultLocation{target, i};
       });
   }
-  void visitCallIndirect(CallIndirect* curr) {
-    // TODO: the table identity could also be used here
-    auto targetType = curr->heapType;
+  template<typename T> void handleIndirectCall(T* curr, HeapType targetType) {
     handleCall(
       curr,
       [&](Index i) {
@@ -525,21 +524,55 @@ struct InfoCollector
         return SignatureResultLocation{targetType, i};
       });
   }
-  void visitCallRef(CallRef* curr) {
-    auto targetType = curr->target->type;
+  template<typename T> void handleIndirectCall(T* curr, Type targetType) {
+    // If the type is unreachable, nothing can be called (and there is no heap
+    // type to get).
     if (targetType != Type::unreachable) {
-      auto heapType = targetType.getHeapType();
-      handleCall(
-        curr,
-        [&](Index i) {
-          return SignatureParamLocation{heapType, i};
-        },
-        [&](Index i) {
-          return SignatureResultLocation{heapType, i};
-        });
+      handleIndirectCall(curr, targetType.getHeapType());
     }
   }
 
+  void visitCall(Call* curr) {
+    Name targetName;
+    if (!Intrinsics(*getModule()).isCallWithoutEffects(curr)) {
+      // This is just a normal call.
+      handleDirectCall(curr, curr->target);
+      return;
+    }
+    // A call-without-effects receives a function reference and calls it, the
+    // same as a CallRef. When we have a flag for non-closed-world, we should
+    // handle this automatically by the reference flowing out to an import,
+    // which is what binaryen intrinsics look like. For now, to support use
+    // cases of a closed world but that also use this intrinsic, handle the
+    // intrinsic specifically here. (Without that, the closed world assumption
+    // makes us ignore the function ref that flows to an import, so we are not
+    // aware that it is actually called.)
+    auto* target = curr->operands.back();
+    if (auto* refFunc = target->dynCast<RefFunc>()) {
+      // We can see exactly where this goes.
+      handleDirectCall(curr, refFunc->func);
+    } else {
+      // We can't see where this goes. We must be pessimistic and assume it
+      // can call anything of the proper type, the same as a CallRef. (We could
+      // look at the possible contents of |target| during the flow, but that
+      // would require special logic like we have for RefCast etc., and the
+      // intrinsics will be lowered away anyhow, so just running after that is
+      // a workaround.)
+      handleIndirectCall(curr, target->type);
+    }
+  }
+  void visitCallIndirect(CallIndirect* curr) {
+    // TODO: the table identity could also be used here
+    // TODO: optimize the call target like CallRef
+    handleIndirectCall(curr, curr->heapType);
+  }
+  void visitCallRef(CallRef* curr) {
+    // TODO: Optimize like RefCast etc.: the values reaching us depend on the
+    //       possible values of |target| (which might be nothing, or might be a
+    //       constant function).
+    handleIndirectCall(curr, curr->target->type);
+  }
+
   // Creates a location for a null of a particular type and adds a root for it.
   // Such roots are where the default value of an i32 local comes from, or the
   // value in a ref.null.
diff --git a/src/passes/RemoveUnusedModuleElements.cpp b/src/passes/RemoveUnusedModuleElements.cpp
index 9268232b4..7116475cd 100644
--- a/src/passes/RemoveUnusedModuleElements.cpp
+++ b/src/passes/RemoveUnusedModuleElements.cpp
@@ -132,7 +132,9 @@ struct ReachabilityAnalyzer : public PostWalker<ReachabilityAnalyzer> {
       // handle this automatically by the reference flowing out to an import,
       // which is what binaryen intrinsics look like. For now, to support use
       // cases of a closed world but that also use this intrinsic, handle the
-      // intrinsic specifically here.
+      // intrinsic specifically here. (Without that, the closed world assumption
+      // makes us ignore the function ref that flows to an import, so we are not
+      // aware that it is actually called.)
       auto* target = curr->operands.back();
       if (auto* refFunc = target->dynCast<RefFunc>()) {
         // We can see exactly where this goes.