PR updates

Author: stevenfontanella

src/passes/GlobalEffects.cpp

@@ -122,24 +122,27 @@ std::map<Function*, FuncInfo> analyzeFuncs(Module& module,
 }
 
 using CallGraphNode = std::variant<Function*, HeapType>;
-using CallGraph =
-  std::unordered_map<CallGraphNode, std::unordered_set<CallGraphNode>>;
 
 /*
- Build a call graph for indirect and direct calls.
+ Call graph for indirect and direct calls.
 
  key (caller) -> value (callee)
- Name         -> Name     : direct call
- Name         -> HeapType : indirect call to the given HeapType
- HeapType     -> Name     : The function `callee` has the type `caller`. The
-                            HeapType may essentially 'call' any of its
-                            potential implementations.
- HeapType     -> HeapType : `callee` is a subtype of `caller`. A call_ref
-                            could target any subtype of the ref, so we need to
-                            aggregate effects of subtypes of the target type.
-
- If we're running in an open world, we only include Name -> Name edges.
+ Function  -> Function : direct call
+ Function  -> HeapType : indirect call to the given HeapType
+ HeapType  -> Function : The function `callee` has the type `caller`. The
+                         HeapType may essentially 'call' any of its
+                         potential implementations.
+ HeapType  -> HeapType : `callee` is a subtype of `caller`. A call_ref
+                         could target any subtype of the ref, so we need to
+                         aggregate effects of subtypes of the target type.
+
+ If we're running in an open world, we only include Function -> Function edges,
+ and don't compute effects for indirect calls, conservatively assuming the
+ worst.
 */
+using CallGraph =
+  std::unordered_map<CallGraphNode, std::unordered_set<CallGraphNode>>;
+
 CallGraph buildCallGraph(const Module& module,
                          const std::map<Function*, FuncInfo>& funcInfos,
                          bool closedWorld) {
@@ -149,7 +152,7 @@ CallGraph buildCallGraph(const Module& module,
   for (const auto& [caller, callerInfo] : funcInfos) {
     auto& callees = callGraph[caller];
 
-    // Name -> Name
+    // Function -> Function
     for (Name calleeFunction : callerInfo.calledFunctions) {
       callees.insert(module.getFunction(calleeFunction));
     }
@@ -158,24 +161,26 @@ CallGraph buildCallGraph(const Module& module,
       continue;
     }
 
-    // Name -> Type
+    // Function -> Type
     allFunctionTypes.insert(caller->type.getHeapType());
     for (HeapType calleeType : callerInfo.indirectCalledTypes) {
       callees.insert(calleeType);
       allFunctionTypes.insert(calleeType);
     }
 
-    // Type -> Name
+    // Type -> Function
     callGraph[caller->type.getHeapType()].insert(caller);
   }
 
   // Type -> Type
   for (HeapType type : allFunctionTypes) {
-    // Not needed but during lookup we expect the key to exist.
+    // Not needed except that during lookup we expect the key to exist.
     callGraph[type];
 
     for (auto super = type.getDeclaredSuperType(); super;
          super = super->getDeclaredSuperType()) {
+      // Don't bother noting supertypes with no functions in it. There are no
+      // effects to aggregate anyway.
       if (allFunctionTypes.contains(*super)) {
         callGraph[*super].insert(type);
       }
@@ -244,7 +249,7 @@ void propagateEffects(const Module& module,
                       const PassOptions& passOptions,
                       std::map<Function*, FuncInfo>& funcInfos,
                       const CallGraph& callGraph) {
-  // We only care about Functions that are roots, not types
+  // 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.
   auto funcNodes = std::views::keys(callGraph) |

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

@@ -19,14 +19,26 @@
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $calls-nop-via-ref (param $ref (ref $nopType))
-    ;; This can only possibly be a nop in closed-world
+    ;; This can only possibly be a nop in closed-world.
     ;; Ideally vacuum could optimize this out but we don't have a way to share
     ;; this information with other passes today.
     ;; For now, we can at least annotate that the call to this function in $f
-    ;; has no effects
+    ;; has no effects.
+    ;; TODO: This call_ref could be marked as having no effects, like the call below.
     (call_ref $nopType (i32.const 1) (local.get $ref))
   )
 
+  ;; CHECK:      (func $calls-nop-via-nullable-ref (type $2) (param $ref (ref null $nopType))
+  ;; CHECK-NEXT:  (call_ref $nopType
+  ;; CHECK-NEXT:   (i32.const 1)
+  ;; CHECK-NEXT:   (local.get $ref)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $calls-nop-via-nullable-ref (param $ref (ref null $nopType))
+    (call_ref $nopType (i32.const 1) (local.get $ref))
+  )
+
+
   ;; CHECK:      (func $f (type $1) (param $ref (ref $nopType))
   ;; CHECK-NEXT:  (nop)
   ;; CHECK-NEXT: )
@@ -35,6 +47,16 @@
     ;; call $nop. We can optimize this call out.
     (call $calls-nop-via-ref (local.get $ref))
   )
+
+  ;; CHECK:      (func $g (type $2) (param $ref (ref null $nopType))
+  ;; CHECK-NEXT:  (call $calls-nop-via-nullable-ref
+  ;; CHECK-NEXT:   (local.get $ref)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $g (param $ref (ref null $nopType))
+    ;; Similar to $f, but we may still trap here because the ref is null.
+    (call $calls-nop-via-nullable-ref (local.get $ref))
+  )
 )
 
 (module
@@ -71,7 +93,7 @@
   ;; CHECK-NEXT:  )
   ;; CHECK-NEXT: )
   (func $f (param $ref (ref $maybe-has-effects))
-    ;; This may be a nop or it may trap depending on the ref
+    ;; This may be a nop or it may trap depending on the ref.
     ;; We don't know so don't optimize it out.
     (call $calls-effectful-function-via-ref (local.get $ref))
   )
@@ -156,7 +178,7 @@
 )
 
 ;; Same as above but this time our reference is the exact supertype
-;; So we know not to aggregate effects from the subtype.
+;; so we know not to aggregate effects from the subtype.
 ;; TODO: this case doesn't optimize today. Add exact ref support in the pass.
 (module
   ;; CHECK:      (type $super (sub (struct)))
@@ -240,7 +262,7 @@
   (func $f (param $ref (ref $only-has-effects-in-not-addressable-function))
     ;; The type $has-effects-but-not-exported doesn't have an address because
     ;; it's not exported and it's never the target of a ref.func.
-    ;; We should be able to determine that $ref can only point to $nop
+    ;; We should be able to determine that $ref can only point to $nop.
     ;; TODO: Only aggregate effects from functions that are addressed.
     (call $calls-type-with-effects-but-not-addressable (local.get $ref))
   )