Use effects for indirect call expressions

src/ir/effects.h

@@ -23,6 +23,7 @@
 #include "ir/intrinsics.h"
 #include "pass.h"
 #include "support/name.h"
+#include "support/utilities.h"
 #include "wasm-traversal.h"
 #include "wasm-type.h"
 #include "wasm.h"
@@ -716,71 +717,50 @@ class EffectAnalyzer {
     }
 
     void visitCall(Call* curr) {
-      // call.without.effects has no effects.
       if (Intrinsics(parent.module).isCallWithoutEffects(curr)) {
+        if (curr->isReturn) {
+          parent.branchesOut = true;
+        }
         return;
       }
 
-      // Get the target's effects, if they exist. Note that we must handle the
-      // case of the function not yet existing (we may be executed in the middle
-      // of a pass, which may have built up calls but not the targets of those
-      // calls; in such a case, we do not find the targets and therefore assume
-      // we know nothing about the effects, which is safe).
-      const EffectAnalyzer* targetEffects = nullptr;
-      if (auto* target = parent.module.getFunctionOrNull(curr->target)) {
-        targetEffects = target->effects.get();
-      }
-
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        // When EH is enabled, any call can throw.
-        if (parent.features.hasExceptionHandling() &&
-            (!targetEffects || targetEffects->throws())) {
-          parent.hasReturnCallThrow = true;
-        }
+      const EffectAnalyzer* bodyEffects = nullptr;
+      if (auto* target = parent.module.getFunctionOrNull(curr->target);
+          target && target->effects) {
+        bodyEffects = target->effects.get();
       }
-
-      if (targetEffects) {
-        // We have effect information for this call target, and can just use
-        // that. The one change we may want to make is to remove throws_, if the
-        // target function throws and we know that will be caught anyhow, the
-        // same as the code below for the general path. We can always filter out
-        // throws for return calls because they are already more precisely
-        // captured by `branchesOut`, which models the return, and
-        // `hasReturnCallThrow`, which models the throw that will happen after
-        // the return.
-        if (targetEffects->throws_ && (parent.tryDepth > 0 || curr->isReturn)) {
-          auto filteredEffects = *targetEffects;
-          filteredEffects.throws_ = false;
-          parent.mergeIn(filteredEffects);
-        } else {
-          // Just merge in all the effects.
-          parent.mergeIn(*targetEffects);
-        }
+      populateEffectsForCall(curr, bodyEffects);
+    }
+    void visitCallIndirect(CallIndirect* curr) {
+      auto* table = parent.module.getTable(curr->table);
+      if (!Type::isSubType(Type(curr->heapType, Nullability::Nullable),
+                           table->type)) {
+        parent.trap = true;
         return;
       }
+      if (table->type.isNullable()) {
+        parent.implicitTrap = true;
+      }
 
-      parent.calls = true;
-      // When EH is enabled, any call can throw. Skip this for return calls
-      // because the throw is already more precisely captured by the combination
-      // of `hasReturnCallThrow` and `branchesOut`.
-      if (parent.features.hasExceptionHandling() && parent.tryDepth == 0 &&
-          !curr->isReturn) {
-        parent.throws_ = true;
+      const EffectAnalyzer* bodyEffects = nullptr;
+      if (auto it = parent.module.typeEffects.find(curr->heapType);
+          it != parent.module.typeEffects.end() && it->second) {
+        bodyEffects = it->second.get();
       }
+      populateEffectsForCall(curr, bodyEffects);
     }
-    void visitCallIndirect(CallIndirect* curr) {
-      parent.calls = true;
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        if (parent.features.hasExceptionHandling()) {
-          parent.hasReturnCallThrow = true;
-        }
+    void visitCallRef(CallRef* curr) {
+      if (trapOnNull(curr->target)) {
+        return;
       }
-      if (parent.features.hasExceptionHandling() &&
-          (parent.tryDepth == 0 && !curr->isReturn)) {
-        parent.throws_ = true;
+
+      const EffectAnalyzer* bodyEffects = nullptr;
+      if (auto it =
+            parent.module.typeEffects.find(curr->target->type.getHeapType());
+          it != parent.module.typeEffects.end() && it->second) {
+        bodyEffects = it->second.get();
       }
+      populateEffectsForCall(curr, bodyEffects);
     }
     void visitLocalGet(LocalGet* curr) {
       parent.localsRead.insert(curr->index);
@@ -1038,22 +1018,6 @@ class EffectAnalyzer {
     void visitTupleExtract(TupleExtract* curr) {}
     void visitRefI31(RefI31* curr) {}
     void visitI31Get(I31Get* curr) { trapOnNull(curr->i31); }
-    void visitCallRef(CallRef* curr) {
-      if (trapOnNull(curr->target)) {
-        return;
-      }
-      if (curr->isReturn) {
-        parent.branchesOut = true;
-        if (parent.features.hasExceptionHandling()) {
-          parent.hasReturnCallThrow = true;
-        }
-      }
-      parent.calls = true;
-      if (parent.features.hasExceptionHandling() &&
-          (parent.tryDepth == 0 && !curr->isReturn)) {
-        parent.throws_ = true;
-      }
-    }
     void visitRefTest(RefTest* curr) {}
 
     void visitRefCast(RefCast* curr) {
@@ -1335,6 +1299,55 @@ class EffectAnalyzer {
         parent.throws_ = true;
       }
     }
+
+  private:
+    // Populate effects of the function's body that were computed from
+    // GlobalEffects. Note that calls may have other effects that aren't
+    // captured by the function body of the target (e.g. a call_ref may trap on
+    // null refs).
+    template<typename CallType>
+    void populateFunctionBodyEffects(const CallType* curr,
+                                     const EffectAnalyzer& funcEffects) {
+      if (curr->isReturn) {
+        if (funcEffects.throws()) {
+          parent.hasReturnCallThrow = true;
+        }
+      }
+
+      if (funcEffects.throws_ && (parent.tryDepth > 0 || curr->isReturn)) {
+        auto filteredEffects = funcEffects;
+        filteredEffects.throws_ = false;
+        parent.mergeIn(filteredEffects);
+      } else {
+        parent.mergeIn(funcEffects);
+      }
+    }
+
+    template<typename CallType>
+    void
+    populateEffectsForCall(const CallType* curr,
+                           NullablePtr<const EffectAnalyzer*> bodyEffects) {
+      if (curr->isReturn) {
+        parent.branchesOut = true;
+      }
+
+      if (bodyEffects) {
+        populateFunctionBodyEffects(curr, *bodyEffects);
+        return;
+      }
+
+      parent.calls = true;
+      // If EH is enabled and we don't have global effects information,
+      // assume that the call body may throw.
+      if (parent.features.hasExceptionHandling()) {
+        if (curr->isReturn) {
+          parent.hasReturnCallThrow = true;
+        }
+        if (parent.tryDepth == 0 && !curr->isReturn) {
+          parent.throws_ = true;
+        }
+      }
+    }
   };
 
 public:

src/ir/type-updating.cpp

@@ -324,6 +324,27 @@ void GlobalTypeRewriter::mapTypes(const TypeMap& oldToNewTypes) {
   for (auto& tag : wasm.tags) {
     tag->type = updater.getNew(tag->type);
   }
+
+  // Update indirect call effects per type.
+  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
+    newTypeEffects;
+  for (auto& [oldType, effects] : wasm.typeEffects) {
+    if (!effects) {
+      continue;
+    }
+
+    auto newType = updater.getNew(oldType);
+    std::shared_ptr<const EffectAnalyzer>& targetEffects =
+      newTypeEffects[newType];
+    if (!targetEffects) {
+      targetEffects = effects;
+    } else {
+      auto merged = std::make_shared<EffectAnalyzer>(*targetEffects);
+      merged->mergeIn(*effects);
+      targetEffects = merged;
+    }
+  }
+  wasm.typeEffects = std::move(newTypeEffects);
 }
 
 void GlobalTypeRewriter::mapTypeNamesAndIndices(const TypeMap& oldToNewTypes) {

src/passes/GlobalEffects.cpp

@@ -24,6 +24,7 @@
 #include "pass.h"
 #include "support/graph_traversal.h"
 #include "support/strongly_connected_components.h"
+#include "support/utilities.h"
 #include "wasm.h"
 
 namespace wasm {
@@ -225,10 +226,13 @@ void mergeMaybeEffects(std::optional<EffectAnalyzer>& dest,
 // - Merge all of the effects of functions within the CC
 // - Also merge the (already computed) effects of each callee CC
 // - Add trap effects for potentially recursive call chains
-void propagateEffects(const Module& module,
-                      const PassOptions& passOptions,
-                      std::map<Function*, FuncInfo>& funcInfos,
-                      const CallGraph& callGraph) {
+void propagateEffects(
+  const Module& module,
+  const PassOptions& passOptions,
+  std::map<Function*, FuncInfo>& funcInfos,
+  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>&
+    typeEffects,
+  const CallGraph& callGraph) {
   // We only care about Functions that are roots, not types.
   // A type would be a root if a function exists with that type, but no-one
   // indirect calls the type.
@@ -317,12 +321,21 @@ void propagateEffects(const Module& module,
     }
 
     // Assign each function's effects to its CC effects.
-    for (Function* f : ccFuncs) {
-      if (!ccEffects) {
-        funcInfos.at(f).effects = UnknownEffects;
-      } else {
-        funcInfos.at(f).effects.emplace(*ccEffects);
-      }
+    for (auto node : cc) {
+      std::visit(overloaded{[&](HeapType type) {
+                              if (ccEffects != UnknownEffects) {
+                                typeEffects[type] =
+                                  std::make_shared<EffectAnalyzer>(*ccEffects);
+                              }
+                            },
+                            [&](Function* f) {
+                              if (!ccEffects) {
+                                funcInfos.at(f).effects = UnknownEffects;
+                              } else {
+                                funcInfos.at(f).effects.emplace(*ccEffects);
+                              }
+                            }},
+                 node);
     }
   }
 }
@@ -346,7 +359,8 @@ struct GenerateGlobalEffects : public Pass {
     auto callGraph =
       buildCallGraph(*module, funcInfos, getPassOptions().closedWorld);
 
-    propagateEffects(*module, getPassOptions(), funcInfos, callGraph);
+    propagateEffects(
+      *module, getPassOptions(), funcInfos, module->typeEffects, callGraph);
 
     copyEffectsToFunctions(funcInfos);
   }

src/support/utilities.h

@@ -94,6 +94,19 @@ class Fatal {
 #define WASM_UNREACHABLE(msg) wasm::handle_unreachable()
 #endif
 
+// Helper to create an invocable with an overloaded operator(), for use with
+// std::visit e.g.
+// std::visit(
+//   overloaded{
+//     [](const A& a) { ... },
+//     [](const B& b) { ... }},
+//   variant)
+template<class... Ts> struct overloaded : Ts... {
+  using Ts::operator()...;
+};
+
+template<typename T> using NullablePtr = T;
+
 } // namespace wasm
 
 #endif // wasm_support_utilities_h

src/wasm.h

@@ -2722,6 +2722,12 @@ class Module {
   std::unordered_map<HeapType, TypeNames> typeNames;
   std::unordered_map<HeapType, Index> typeIndices;
 
+  // Potential effects for bodies of indirect calls to this type.
+  // TODO: Use Type instead of HeapType to account for nullability and
+  // exactness.
+  std::unordered_map<HeapType, std::shared_ptr<const EffectAnalyzer>>
+    typeEffects;
+
   MixedArena allocator;
 
 private:

test/lit/passes/global-effects-closed-world-tnh.wast

@@ -1,5 +1,5 @@
 ;; NOTE: Assertions have been generated by update_lit_checks.py and should not be edited.
-;; RUN: foreach %s %t wasm-opt -all --closed-world --traps-never-happen --generate-global-effects --vacuum -S -o - | filecheck %s
+;; RUN: wasm-opt %s -all --closed-world --traps-never-happen --generate-global-effects --vacuum -S -o - | filecheck %s
 
 ;; Tests for aggregating effects from indirect calls in GlobalEffects when
 ;; --closed-world is true. Continued from global-effects-closed-world.wast.
@@ -8,6 +8,8 @@
   ;; CHECK:      (type $nopType (func (param i32)))
   (type $nopType (func (param i32)))
 
+  (table 1 1 funcref)
+
   ;; CHECK:      (func $nop (type $nopType) (param $0 i32)
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
@@ -16,22 +18,19 @@
   )
 
   ;; CHECK:      (func $calls-nop-via-nullable-ref (type $1) (param $ref (ref null $nopType))
-  ;; CHECK-NEXT:  (call_ref $nopType
-  ;; CHECK-NEXT:   (i32.const 1)
-  ;; CHECK-NEXT:   (local.get $ref)
-  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
   (func $calls-nop-via-nullable-ref (param $ref (ref null $nopType))
+    ;; We would trap if $ref is null, but otherwise this has no effects.
     (call_ref $nopType (i32.const 1) (local.get $ref))
   )
 
-  ;; CHECK:      (func $f (type $1) (param $ref (ref null $nopType))
+  ;; CHECK:      (func $calls-nop-via-ref (type $2)
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
-  (func $f (param $ref (ref null $nopType))
-    ;; The only possible implementation of $nopType has no effects.
-    ;; $calls-nop-via-nullable-ref may trap from a null reference, but
-    ;; --traps-never-happen is enabled, so we're free to optimize this out.
-    (call $calls-nop-via-nullable-ref (local.get $ref))
+  (func $calls-nop-via-ref
+    ;; We may trap due to index out of bounds or the function type not matching
+    ;; the table, but otherwise this has no possible effects.
+    (call_indirect (type $nopType) (i32.const 1) (i32.const 0))
   )
 )