From 757d6a21a3c5626cfaa9dac28c28555748b93a6b Mon Sep 17 00:00:00 2001
From: Alon Zakai <azakai@google.com>
Date: Fri, 7 Apr 2023 09:49:31 -0700
Subject: [GUFA] Refine global types during flow (#5639)

Previously (ref.as_non_null (global.get ..)) would return the global with no changes,
and if the global was nullable then the type didn't match the output, which hit an
assertion (where GUFA checks that the contents match the declared type in the wasm).

To fix this, refine global types, that is, the type we track on GlobalInfo may be more
refined than the global itself. In the above example, if the global is nullable then
the GlobalInfo would point to that global but have a non-nullable type.

In fact the code was already prepared for this, and few changes were needed.
---
 src/ir/possible-contents.cpp | 26 +++++++++++++++++++-------
 src/ir/possible-contents.h   | 18 +++++++++++++-----
 2 files changed, 32 insertions(+), 12 deletions(-)

(limited to 'src')

diff --git a/src/ir/possible-contents.cpp b/src/ir/possible-contents.cpp
index b2a0b417a..83762a458 100644
--- a/src/ir/possible-contents.cpp
+++ b/src/ir/possible-contents.cpp
@@ -197,18 +197,17 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
     return;
   }
 
-  if (isLiteral() || isGlobal()) {
+  if (isLiteral()) {
     // The information about the value being identical to a particular literal
-    // or immutable global is not removed by intersection, if the type is in the
-    // cone we are intersecting with.
+    // is not removed by intersection, if the type is in the cone we are
+    // intersecting with.
     if (isSubType) {
       return;
     }
 
-    // The type must change, so continue down to the generic code path.
-    // TODO: for globals we could perhaps refine the type here, but then the
-    //       type on GlobalInfo would not match the module, so that needs some
-    //       refactoring.
+    // The type must change in a nontrivial manner, so continue down to the
+    // generic code path. This will stop being a Literal. TODO: can we do better
+    // here?
   }
 
   // Intersect the cones, as there is no more specific information we can use.
@@ -226,6 +225,14 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
     newHeapType = heapType;
   }
 
+  // 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;
+  if (isGlobal()) {
+    globalName = getGlobal();
+  }
+
   auto newType = Type(newHeapType, nullability);
 
   // By assumption |other| has full depth. Consider the other cone in |this|.
@@ -261,6 +268,11 @@ void PossibleContents::intersectWithFullCone(const PossibleContents& other) {
 
     value = ConeType{newType, newDepth};
   }
+
+  if (globalName) {
+    // Restore the global but keep the new and refined type.
+    value = GlobalInfo{*globalName, getType()};
+  }
 }
 
 bool PossibleContents::haveIntersection(const PossibleContents& a,
diff --git a/src/ir/possible-contents.h b/src/ir/possible-contents.h
index b7c9bfafd..b4326688e 100644
--- a/src/ir/possible-contents.h
+++ b/src/ir/possible-contents.h
@@ -66,10 +66,15 @@ class PossibleContents {
 
   struct GlobalInfo {
     Name name;
-    // The type of the global in the module. We stash this here so that we do
-    // not need to pass around a module all the time.
-    // TODO: could we save size in this variant if we did pass around the
-    //       module?
+    // The type of contents. Note that this may not match the type of the
+    // global, if we were filtered. For example:
+    //
+    //  (ref.as_non_null
+    //    (global.get $nullable-global)
+    //  )
+    //
+    // The contents flowing out will be a Global, but of a non-nullable type,
+    // unlike the original global.
     Type type;
     bool operator==(const GlobalInfo& other) const {
       return name == other.name && type == other.type;
@@ -287,6 +292,9 @@ public:
       return builder.makeConstantExpression(getLiteral());
     } else {
       auto name = 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);
     }
   }
@@ -321,7 +329,7 @@ public:
         o << " HT: " << h;
       }
     } else if (isGlobal()) {
-      o << "GlobalInfo $" << getGlobal();
+      o << "GlobalInfo $" << getGlobal() << " T: " << getType();
     } else if (auto* coneType = std::get_if<ConeType>(&value)) {
       auto t = coneType->type;
       o << "ConeType " << t;
-- 
cgit v1.2.3