GUFA: Represent imported functions as globals (#8025)

The meaning of Global in PossibleContents changes from "an actual wasm
Global" to "an immutable global wasm thing, either a Global or a Function."
Imported wasm Functions are effectively immutable values in the global
scope - not concrete, specifically-known functions - so this is more correct.

In particular, two imported Globals (of compatible types) might or might not
be the same, and likewise with imported Functions. And that is not the case
for defined Functions - they are definitely different.

This does not fix the issues related to imported function type handling -
#7993 does that - but this refactoring makes it possible to properly
optimize imported functions afterward.

This does make the PossibleContents object grow from 32 to 40 bytes,
which might be why this adds 3-4% overhead to GUFA, but I don't see a
good way to avoid that, unfortunately. Keeping our ability to optimize
imported functions might be worth that 3-4% (we recently saw a few digits
improvement due to properly optimizing imported externs, for example,
from #8005).

Author: kripken

src/ir/possible-contents.cpp

@@ -208,9 +208,9 @@ void PossibleContents::intersect(const PossibleContents& other) {
   // Note the global's information, if we started as a global. In that case, the
   // code below will refine our type but we can remain a global, which we will
   // accomplish by restoring our global status at the end.
-  std::optional<Name> globalName;
+  std::optional<GlobalInfo> global;
   if (isGlobal()) {
-    globalName = getGlobal();
+    global = getGlobal();
   }
 
   if (hasFullCone() && other.hasFullCone()) {
@@ -230,9 +230,9 @@ void PossibleContents::intersect(const PossibleContents& other) {
     value = ConeType{newType, std::min(newDepth, otherNewDepth)};
   }
 
-  if (globalName) {
+  if (global) {
     // Restore the global but keep the new and refined type.
-    value = GlobalInfo{*globalName, getType()};
+    value = GlobalInfo{global->name, global->kind, getType()};
   }
 }
 
@@ -641,9 +641,17 @@ struct InfoCollector
     addRoot(curr);
   }
   void visitRefFunc(RefFunc* curr) {
-    addRoot(curr,
-            PossibleContents::literal(
-              Literal::makeFunc(curr->func, curr->type.getHeapType())));
+    if (!getModule()->getFunction(curr->func)->imported()) {
+      // This is not imported, so we know the exact function literal.
+      addRoot(curr,
+              PossibleContents::literal(
+                Literal::makeFunc(curr->func, curr->type.getHeapType())));
+    } else {
+      // This is imported, so it is effectively a global.
+      addRoot(curr,
+              PossibleContents::global(
+                curr->func, ExternalKind::Function, curr->type));
+    }
 
     // The presence of a RefFunc indicates the function may be called
     // indirectly, so add the relevant connections for this particular function.
@@ -2836,7 +2844,8 @@ void Flower::filterGlobalContents(PossibleContents& contents,
     // a cone/exact type *and* that something is equal to a global, in some
     // cases. See https://github.com/WebAssembly/binaryen/pull/5083
     if (contents.isMany() || contents.isConeType()) {
-      contents = PossibleContents::global(global->name, global->type);
+      contents = PossibleContents::global(
+        global->name, ExternalKind::Global, global->type);
 
       // TODO: We could do better here, to set global->init->type instead of
       //       global->type, or even the contents.getType() - either of those

src/ir/possible-contents.h

@@ -40,10 +40,11 @@ namespace wasm {
 //
 //  * Literal:         One possible constant value like an i32 of 42.
 //
-//  * Global:          The name of a global whose value is here. We do not know
-//                     the actual value at compile time, but we know it is equal
-//                     to that global. Typically we can only infer this for
-//                     immutable globals.
+//  * Global:          An immutable global value, something who we can identify
+//                     but do not know the actual value of at runtime. This can
+//                     be either a wasm (immutable) Global, or an imported wasm
+//                     Function (which is effectively the same: we can refer to
+//                     it, but do not know what is being imported there).
 //
 //  * ConeType:        Any possible value of a particular type, and a possible
 //                     "cone" of a certain depth below it. If the depth is 0
@@ -85,6 +86,7 @@ class PossibleContents {
 
   struct GlobalInfo {
     Name name;
+    ExternalKind kind;
     // The type of contents. Note that this may not match the type of the
     // global, if we were filtered. For example:
     //
@@ -99,7 +101,7 @@ class PossibleContents {
     //       way. In principle, not having depth info can lead to loss of
     //       precision.
     bool operator==(const GlobalInfo& other) const {
-      return name == other.name && type == other.type;
+      return name == other.name && kind == other.kind && type == other.type;
     }
   };
 
@@ -144,8 +146,8 @@ class PossibleContents {
 
   static PossibleContents none() { return PossibleContents{None()}; }
   static PossibleContents literal(Literal c) { return PossibleContents{c}; }
-  static PossibleContents global(Name name, Type type) {
-    return PossibleContents{GlobalInfo{name, type}};
+  static PossibleContents global(Name name, ExternalKind kind, Type type) {
+    return PossibleContents{GlobalInfo{name, kind, type}};
   }
   // Helper for a cone type with depth 0, i.e., an exact type.
   static PossibleContents exactType(Type type) {
@@ -216,9 +218,9 @@ class PossibleContents {
     return std::get<Literal>(value);
   }
 
-  Name getGlobal() const {
+  GlobalInfo getGlobal() const {
     assert(isGlobal());
-    return std::get<GlobalInfo>(value).name;
+    return std::get<GlobalInfo>(value);
   }
 
   bool isNull() const { return isLiteral() && getLiteral().isNull(); }
@@ -316,11 +318,18 @@ class PossibleContents {
     if (isLiteral()) {
       return builder.makeConstantExpression(getLiteral());
     } else {
-      auto name = getGlobal();
+      auto info = getGlobal();
       // Note that we load the type from the module, rather than use the type
       // in the GlobalInfo, as that type may not match the global (see comment
       // in the GlobalInfo declaration above).
-      return builder.makeGlobalGet(name, wasm.getGlobal(name)->type);
+      if (info.kind == ExternalKind::Global) {
+        return builder.makeGlobalGet(info.name,
+                                     wasm.getGlobal(info.name)->type);
+      } else {
+        assert(info.kind == ExternalKind::Function);
+        return builder.makeRefFunc(
+          info.name, wasm.getFunction(info.name)->type.getHeapType());
+      }
     }
   }
 
@@ -352,6 +361,7 @@ class PossibleContents {
       rehash(ret, getLiteral());
     } else if (auto* global = std::get_if<GlobalInfo>(&value)) {
       rehash(ret, global->name);
+      rehash(ret, global->kind);
       rehash(ret, global->type);
     } else if (auto* coneType = std::get_if<ConeType>(&value)) {
       rehash(ret, coneType->type);
@@ -374,7 +384,9 @@ class PossibleContents {
         o << " HT: " << h;
       }
     } else if (isGlobal()) {
-      o << "GlobalInfo $" << getGlobal() << " T: " << getType();
+      auto info = getGlobal();
+      o << "GlobalInfo $" << info.name << " K: " << int(info.kind)
+        << " T: " << getType();
     } else if (auto* coneType = std::get_if<ConeType>(&value)) {
       auto t = coneType->type;
       o << "ConeType " << t;

test/gtest/possible-contents.cpp

@@ -80,14 +80,22 @@ class PossibleContentsTest : public testing::Test {
     PossibleContents::literal(Literal::makeNull(HeapType::i31));
 
   PossibleContents i32Global1 =
-    PossibleContents::global("i32Global1", Type::i32);
+    PossibleContents::global("i32Global1", ExternalKind::Global, Type::i32);
   PossibleContents i32Global2 =
-    PossibleContents::global("i32Global2", Type::i32);
-  PossibleContents f64Global = PossibleContents::global("f64Global", Type::f64);
-  PossibleContents anyGlobal = PossibleContents::global("anyGlobal", anyref);
-  PossibleContents funcGlobal = PossibleContents::global("funcGlobal", funcref);
-  PossibleContents nonNullFuncGlobal =
-    PossibleContents::global("funcGlobal", Type(HeapType::func, NonNullable));
+    PossibleContents::global("i32Global2", ExternalKind::Global, Type::i32);
+  PossibleContents f64Global =
+    PossibleContents::global("f64Global", ExternalKind::Global, Type::f64);
+  PossibleContents anyGlobal =
+    PossibleContents::global("anyGlobal", ExternalKind::Global, anyref);
+  PossibleContents funcGlobal =
+    PossibleContents::global("funcGlobal", ExternalKind::Global, funcref);
+  PossibleContents nonNullFuncGlobal = PossibleContents::global(
+    "funcGlobal", ExternalKind::Global, Type(HeapType::func, NonNullable));
+
+  PossibleContents importedFunc1 = PossibleContents::global(
+    "impfunc1", ExternalKind::Function, Type(HeapType::func, NonNullable));
+  PossibleContents importedFunc2 = PossibleContents::global(
+    "impfunc2", ExternalKind::Function, Type(HeapType::func, NonNullable));
 
   PossibleContents nonNullFunc = PossibleContents::literal(
     Literal::makeFunc("func", Signature(Type::none, Type::none)));
@@ -114,6 +122,8 @@ class PossibleContentsTest : public testing::Test {
   PossibleContents coneAnyref = PossibleContents::coneType(anyref);
   PossibleContents coneFuncref = PossibleContents::coneType(funcref);
   PossibleContents coneFuncref1 = PossibleContents::coneType(funcref, 1);
+  PossibleContents coneNonNullFuncref =
+    PossibleContents::coneType(Type(HeapType::func, NonNullable));
 };
 
 TEST_F(PossibleContentsTest, TestComparisons) {
@@ -135,6 +145,9 @@ TEST_F(PossibleContentsTest, TestComparisons) {
   assertNotEqualSymmetric(i32Global1, exactI32);
   assertNotEqualSymmetric(i32Global1, many);
 
+  assertEqualSymmetric(importedFunc1, importedFunc1);
+  assertNotEqualSymmetric(importedFunc1, importedFunc2);
+
   assertEqualSymmetric(exactI32, exactI32);
   assertNotEqualSymmetric(exactI32, exactAnyref);
   assertNotEqualSymmetric(exactI32, many);
@@ -151,6 +164,23 @@ TEST_F(PossibleContentsTest, TestComparisons) {
   assertNotEqualSymmetric(exactNonNullAnyref, exactAnyref);
 }
 
+TEST_F(PossibleContentsTest, TestComparisonsGlobals) {
+  // Check if two PossibleContents::global, one a wasm Global and one a wasm
+  // Function, and equal in their names and types, are still understood to be
+  // non-equal: the |kind| field differentiates them.
+
+  PossibleContents wasmGlobal = PossibleContents::global(
+    "foo", ExternalKind::Global, Type(HeapType::func, NonNullable));
+  PossibleContents wasmFunction = PossibleContents::global(
+    "foo", ExternalKind::Function, Type(HeapType::func, NonNullable));
+
+  assertNotEqualSymmetric(wasmGlobal, wasmFunction);
+
+  // But they are equal to themselves, of course.
+  assertEqualSymmetric(wasmGlobal, wasmGlobal);
+  assertEqualSymmetric(wasmFunction, wasmFunction);
+}
+
 TEST_F(PossibleContentsTest, TestHash) {
   // Hashes should be deterministic.
   EXPECT_EQ(none.hash(), none.hash());
@@ -257,6 +287,10 @@ TEST_F(PossibleContentsTest, TestCombinations) {
 
   assertCombination(anyGlobal, anyNull, coneAnyref);
   assertCombination(anyGlobal, i31Null, coneAnyref);
+
+  // Imported functions.
+  assertCombination(importedFunc1, importedFunc1, importedFunc1);
+  assertCombination(importedFunc1, importedFunc2, coneNonNullFuncref);
 }
 
 static PassOptions options;
@@ -338,6 +372,13 @@ TEST_F(PossibleContentsTest, TestIntersection) {
 
   // Separate hierarchies.
   assertLackIntersection(funcGlobal, anyGlobal);
+
+  // Imported functions.
+  assertHaveIntersection(importedFunc1, importedFunc1);
+  assertHaveIntersection(importedFunc1, exactFuncSignatureType);
+  assertHaveIntersection(importedFunc1, exactNonNullFuncSignatureType);
+  assertHaveIntersection(importedFunc1, importedFunc2);
+  assertHaveIntersection(importedFunc1, funcGlobal);
 }
 
 TEST_F(PossibleContentsTest, TestIntersectWithCombinations) {
@@ -484,6 +525,8 @@ TEST_F(PossibleContentsTest, TestIntersectWithCombinations) {
                                                   funcGlobal,
                                                   nonNullFuncGlobal,
                                                   nonNullFunc,
+                                                  importedFunc1,
+                                                  importedFunc2,
                                                   exactI32,
                                                   exactAnyref,
                                                   exactFuncref,
@@ -785,9 +828,10 @@ TEST_F(PossibleContentsTest, TestStructCones) {
     nonNullFunc, PossibleContents::coneType(signature), nonNullFunc);
 
   // Filter a global to a more specific type.
-  assertIntersection(funcGlobal,
-                     PossibleContents::coneType(signature),
-                     PossibleContents::global("funcGlobal", signature));
+  assertIntersection(
+    funcGlobal,
+    PossibleContents::coneType(signature),
+    PossibleContents::global("funcGlobal", ExternalKind::Global, signature));
 
   // Filter a global's nullability only.
   auto nonNullFuncRef = Type(HeapType::func, NonNullable);
@@ -814,6 +858,13 @@ TEST_F(PossibleContentsTest, TestStructCones) {
   assertIntersection(funcGlobal, none, none);
   assertIntersection(PossibleContents::coneType(signature), none, none);
 
+  // Imported functions. TODO: These are not yet supported, and assert instead.
+  //  assertIntersection(
+  //    importedFunc1, importedFunc1, importedFunc1);
+  //  assertIntersection(
+  //    importedFunc1, PossibleContents::coneType(nonNullFuncRef),
+  //    importedFunc1);
+
   // Subcontents. This API only supports the case where one of the inputs is a
   // full cone type.
   // First, compare exact types to such a cone.

test/lit/passes/gufa.wast

@@ -1154,3 +1154,34 @@
     )
   )
 )
+
+;; Imported functions can be inferred.
+(module
+  ;; CHECK:      (type $0 (func))
+
+  ;; CHECK:      (type $1 (func (result funcref)))
+
+  ;; CHECK:      (import "" "" (func $f (type $0)))
+  (import "" "" (func $f))
+
+  ;; CHECK:      (elem declare func $f)
+
+  ;; CHECK:      (export "test" (func $test))
+
+  ;; CHECK:      (func $test (type $1) (result funcref)
+  ;; CHECK-NEXT:  (local $temp funcref)
+  ;; CHECK-NEXT:  (local.set $temp
+  ;; CHECK-NEXT:   (ref.func $f)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (ref.func $f)
+  ;; CHECK-NEXT: )
+  (func $test (export "test") (result funcref)
+    (local $temp funcref)
+    (local.set $temp
+      (ref.func $f)
+    )
+    ;; This will become a ref.func $f.
+    (local.get $temp)
+  )
+)
+