Skip to content

Commit 0d9b750

Browse files
authored
[GC] Fix assertion in GlobalTypeOptimization about public super (WebAssembly#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.
1 parent bc36c02 commit 0d9b750

File tree

2 files changed

+81
-9
lines changed

2 files changed

+81
-9
lines changed

src/passes/GlobalTypeOptimization.cpp

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -311,15 +311,10 @@ struct GlobalTypeOptimization : public Pass {
311311
keptFieldsNotInSuper.push_back(i);
312312
}
313313
} else {
314-
// The super kept this field, so we must keep it as well. The
315-
// propagation analysis above ensures that we and the super are in
316-
// agreement on keeping it (the reasons that prevent optimization
317-
// propagate to both), except for the corner case of the parent
318-
// being public but us being private (the propagation does not
319-
// take into account visibility).
320-
assert(
321-
!removableIndexes.count(i) ||
322-
(publicTypesSet.count(*super) && !publicTypesSet.count(type)));
314+
// The super kept this field, so we must keep it as well. This can
315+
// happen when we need the field in both, but also in the corner
316+
// case where we don't need the field but the super is public.
317+
323318
// We need to keep it at the same index so we remain compatible.
324319
indexesAfterRemoval[i] = superIndex;
325320
// Update |next| to refer to the next available index. Due to

test/lit/passes/gto-removals.wast

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1495,3 +1495,80 @@
14951495
)
14961496
)
14971497
)
1498+
1499+
;; The type $A is public because it is on an exported global. As a result we
1500+
;; cannot remove the unused i32 field from its child or grandchild.
1501+
(module
1502+
;; CHECK: (type $A (sub (struct (field (mut i32)))))
1503+
(type $A (sub (struct (field (mut i32)))))
1504+
;; CHECK: (type $B (sub $A (struct (field (mut i32)))))
1505+
(type $B (sub $A (struct (field (mut i32)))))
1506+
;; CHECK: (type $C (sub $B (struct (field (mut i32)))))
1507+
(type $C (sub $B (struct (field (mut i32)))))
1508+
1509+
;; Use $C so it isn't removed trivially, which also keeps $B alive as its
1510+
;; super.
1511+
;; CHECK: (global $global (ref $A) (struct.new_default $C))
1512+
(global $global (ref $A) (struct.new_default $C))
1513+
1514+
;; CHECK: (export "global" (global $global))
1515+
(export "global" (global $global))
1516+
)
1517+
1518+
;; As above, but now there is an f64 field on $C that can be removed, since it
1519+
;; is not on the parents.
1520+
(module
1521+
;; CHECK: (type $A (sub (struct (field (mut i32)))))
1522+
(type $A (sub (struct (field (mut i32)))))
1523+
;; CHECK: (rec
1524+
;; CHECK-NEXT: (type $B (sub $A (struct (field (mut i32)))))
1525+
(type $B (sub $A (struct (field (mut i32)))))
1526+
;; CHECK: (type $C (sub $B (struct (field (mut i32)))))
1527+
(type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
1528+
1529+
;; CHECK: (global $global (ref $A) (struct.new_default $C))
1530+
(global $global (ref $A) (struct.new_default $C))
1531+
1532+
;; CHECK: (export "global" (global $global))
1533+
(export "global" (global $global))
1534+
)
1535+
1536+
;; As above, but the f64 field is now on $B as well. We can still remove it.
1537+
(module
1538+
;; CHECK: (type $A (sub (struct (field (mut i32)))))
1539+
(type $A (sub (struct (field (mut i32)))))
1540+
;; CHECK: (rec
1541+
;; CHECK-NEXT: (type $B (sub $A (struct (field (mut i32)))))
1542+
(type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
1543+
;; CHECK: (type $C (sub $B (struct (field (mut i32)))))
1544+
(type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
1545+
1546+
;; CHECK: (global $global (ref $A) (struct.new_default $C))
1547+
(global $global (ref $A) (struct.new_default $C))
1548+
1549+
;; CHECK: (export "global" (global $global))
1550+
(export "global" (global $global))
1551+
)
1552+
1553+
;; As above, but now $B is public as well. Now we cannot remove the f64.
1554+
(module
1555+
;; CHECK: (type $A (sub (struct (field (mut i32)))))
1556+
(type $A (sub (struct (field (mut i32)))))
1557+
;; CHECK: (type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
1558+
(type $B (sub $A (struct (field (mut i32)) (field (mut f64)))))
1559+
;; CHECK: (type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
1560+
(type $C (sub $B (struct (field (mut i32)) (field (mut f64)))))
1561+
1562+
;; CHECK: (global $global (ref $A) (struct.new_default $C))
1563+
(global $global (ref $A) (struct.new_default $C))
1564+
1565+
;; CHECK: (global $globalB (ref $B) (struct.new_default $C))
1566+
(global $globalB (ref $B) (struct.new_default $C))
1567+
1568+
;; CHECK: (export "global" (global $global))
1569+
(export "global" (global $global))
1570+
1571+
;; CHECK: (export "globalB" (global $globalB))
1572+
(export "globalB" (global $globalB))
1573+
)
1574+

0 commit comments

Comments
 (0)