[GC] Ignore public types in GlobalTypeOptimization (#7017)

TypeUpdater which it uses internally already does so, but we must also
ignore such types earlier, and make no other modifications to them.

Helps #7015

2 files changed, 106 insertions, 3 deletions

diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
index dac6fd7b6..1f0da2595 100644
--- a/src/passes/GlobalTypeOptimization.cpp
+++ b/src/passes/GlobalTypeOptimization.cpp
@@ -23,6 +23,7 @@
 //
 
 #include "ir/localize.h"
+#include "ir/module-utils.h"
 #include "ir/ordering.h"
 #include "ir/struct-utils.h"
 #include "ir/subtypes.h"
@@ -167,13 +168,18 @@ struct GlobalTypeOptimization : public Pass {
     auto dataFromSupersMap = std::move(combinedSetGetInfos);
     propagator.propagateToSubTypes(dataFromSupersMap);
 
+    // Find the public types, which we must not modify.
+    auto publicTypes = ModuleUtils::getPublicHeapTypes(*module);
+    std::unordered_set<HeapType> publicTypesSet(publicTypes.begin(),
+                                                publicTypes.end());
+
     // Process the propagated info. We look at supertypes first, as the order of
     // fields in a supertype is a constraint on what subtypes can do. That is,
     // we decide for each supertype what the optimal order is, and consider that
     // fixed, and then subtypes can decide how to sort fields that they append.
     for (auto type :
          HeapTypeOrdering::supertypesFirst(propagator.subTypes.types)) {
-      if (!type.isStruct()) {
+      if (!type.isStruct() || publicTypesSet.count(type)) {
         continue;
       }
       auto& fields = type.getStruct().fields;
@@ -291,8 +297,15 @@ struct GlobalTypeOptimization : public Pass {
                 keptFieldsNotInSuper.push_back(i);
               }
             } else {
-              // The super kept this field, so we must keep it as well.
-              assert(!removableIndexes.count(i));
+              // The super kept this field, so we must keep it as well. The
+              // propagation analysis above ensures that we and the super are in
+              // agreement on keeping it (the reasons that prevent optimization
+              // propagate to both), except for the corner case of the parent
+              // being public but us being private (the propagation does not
+              // take into account visibility).
+              assert(
+                !removableIndexes.count(i) ||
+                (publicTypesSet.count(*super) && !publicTypesSet.count(type)));
               // We need to keep it at the same index so we remain compatible.
               indexesAfterRemoval[i] = superIndex;
               // Update |next| to refer to the next available index. Due to
diff --git a/test/lit/passes/gto-removals.wast b/test/lit/passes/gto-removals.wast
index c2ac66a57..61396cf8f 100644
--- a/test/lit/passes/gto-removals.wast
+++ b/test/lit/passes/gto-removals.wast
@@ -1405,3 +1405,93 @@
     )
   )
 )
+
+;; Public types cannot be optimized. The function type here is public as the
+;; function is exported, and so the entire rec group is public, and cannot be
+;; modified. We cannot even optimize $child3 which is outside of the rec group,
+;; because its parent is inside. However, we can optimize $unrelated which is
+;; unrelated to them (and so we can remove the field there).
+(module
+  (rec
+    ;; CHECK:      (rec
+    ;; CHECK-NEXT:  (type $parent (sub (struct (field (ref func)))))
+    (type $parent (sub (struct (field (ref func)))))
+    ;; CHECK:       (type $child1 (sub $parent (struct (field (ref func)))))
+    (type $child1 (sub $parent (struct (field (ref func)))))
+    ;; CHECK:       (type $child2 (sub $parent (struct (field (ref func)))))
+    (type $child2 (sub $parent (struct (field (ref func)))))
+
+    ;; CHECK:       (type $func (func (param (ref $child2))))
+    (type $func (func (param $child2 (ref $child2))))
+  )
+
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $unrelated (sub (struct)))
+
+  ;; CHECK:       (type $child3 (sub $parent (struct (field (ref func)))))
+  (type $child3 (sub $parent (struct (field (ref func)))))
+
+  (type $unrelated (sub (struct (field (ref func)))))
+
+  ;; CHECK:      (elem declare func $func)
+
+  ;; CHECK:      (export "func" (func $func))
+
+  ;; CHECK:      (func $func (type $func) (param $child2 (ref $child2))
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new $parent
+  ;; CHECK-NEXT:    (ref.func $func)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new $child1
+  ;; CHECK-NEXT:    (ref.func $func)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new $child2
+  ;; CHECK-NEXT:    (ref.func $func)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new $child3
+  ;; CHECK-NEXT:    (ref.func $func)
+  ;; CHECK-NEXT:   )
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT:  (drop
+  ;; CHECK-NEXT:   (struct.new_default $unrelated)
+  ;; CHECK-NEXT:  )
+  ;; CHECK-NEXT: )
+  (func $func (export "func") (type $func) (param $child2 (ref $child2))
+    ;; Create all the types. Note that the value here is non-nullable, as is the
+    ;; field, so if we remove the field by mistake in GTO but leave it during
+    ;; TypeUpdater, we'd error (on providing a default value for a non-nullable
+    ;; field).
+    (drop
+      (struct.new $parent
+        (ref.func $func)
+      )
+    )
+    (drop
+      (struct.new $child1
+        (ref.func $func)
+      )
+    )
+    (drop
+      (struct.new $child2
+        (ref.func $func)
+      )
+    )
+    (drop
+      (struct.new $child3
+        (ref.func $func)
+      )
+    )
+    ;; We can optimize this one, and no other.
+    (drop
+      (struct.new $unrelated
+        (ref.func $func)
+      )
+    )
+  )
+)