[GC] Fix assertion in GlobalTypeOptimization about public super (#7026)

We only checked for the case of the immediate super being public while we are private, but it might be a grandsuper instead. That is, any ancestor that is public will prevent GTO from removing a field (since we can only add fields on top of our ancestors). Also, the ancestors might not all have the field, which would add more complexity to that particular assertion, so just remove it, and add comprehensive tests.
WebAssembly · Oct 22, 2024 · 0d9b750 · 0d9b750
1 parent bc36c02
commit 0d9b750
Show file tree

Hide file tree

Showing 2 changed files with 81 additions and 9 deletions.
diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp
@@ -311,15 +311,10 @@ struct GlobalTypeOptimization : public Pass {
                 keptFieldsNotInSuper.push_back(i);
               }
             } else {
-              // 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)));
+              // The super kept this field, so we must keep it as well. This can
+              // happen when we need the field in both, but also in the corner
+              // case where we don't need the field but the super is public.
+
               // 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
@@ -1495,3 +1495,80 @@
     )
   )
 )
+
+;; The type $A is public because it is on an exported global. As a result we
+;; cannot remove the unused i32 field from its child or grandchild.
+(module
+  ;; CHECK:      (type $A (sub (struct (field (mut i32)))))
+  (type $A (sub (struct (field (mut i32)))))
+  ;; CHECK:      (type $B (sub $A (struct (field (mut i32)))))
+  (type $B (sub $A (struct (field (mut i32)))))
+  ;; CHECK:      (type $C (sub $B (struct (field (mut i32)))))
+  (type $C (sub $B (struct (field (mut i32)))))
+
+  ;; Use $C so it isn't removed trivially, which also keeps $B alive as its
+  ;; super.
+  ;; CHECK:      (global $global (ref $A) (struct.new_default $C))
+  (global $global (ref $A) (struct.new_default $C))
+
+  ;; CHECK:      (export "global" (global $global))
+  (export "global" (global $global))
+)
+
+;; As above, but now there is an f64 field on $C that can be removed, since it
+;; is not on the parents.
+(module
+  ;; CHECK:      (type $A (sub (struct (field (mut i32)))))
+  (type $A (sub (struct (field (mut i32)))))
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $B (sub $A (struct (field (mut i32)))))
+  (type $B (sub $A (struct (field (mut i32)))))
+  ;; CHECK:       (type $C (sub $B (struct (field (mut i32)))))
+  (type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
+
+  ;; CHECK:      (global $global (ref $A) (struct.new_default $C))
+  (global $global (ref $A) (struct.new_default $C))
+
+  ;; CHECK:      (export "global" (global $global))
+  (export "global" (global $global))
+)
+
+;; As above, but the f64 field is now on $B as well. We can still remove it.
+(module
+  ;; CHECK:      (type $A (sub (struct (field (mut i32)))))
+  (type $A (sub (struct (field (mut i32)))))
+  ;; CHECK:      (rec
+  ;; CHECK-NEXT:  (type $B (sub $A (struct (field (mut i32)))))
+  (type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
+  ;; CHECK:       (type $C (sub $B (struct (field (mut i32)))))
+  (type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
+
+  ;; CHECK:      (global $global (ref $A) (struct.new_default $C))
+  (global $global (ref $A) (struct.new_default $C))
+
+  ;; CHECK:      (export "global" (global $global))
+  (export "global" (global $global))
+)
+
+;; As above, but now $B is public as well. Now we cannot remove the f64.
+(module
+  ;; CHECK:      (type $A (sub (struct (field (mut i32)))))
+  (type $A (sub (struct (field (mut i32)))))
+  ;; CHECK:      (type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
+  (type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
+  ;; CHECK:      (type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
+  (type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
+
+  ;; CHECK:      (global $global (ref $A) (struct.new_default $C))
+  (global $global (ref $A) (struct.new_default $C))
+
+  ;; CHECK:      (global $globalB (ref $B) (struct.new_default $C))
+  (global $globalB (ref $B) (struct.new_default $C))
+
+  ;; CHECK:      (export "global" (global $global))
+  (export "global" (global $global))
+
+  ;; CHECK:      (export "globalB" (global $globalB))
+  (export "globalB" (global $globalB))
+)
+