Skip to content

Commit 245d915

Browse files
authored
[GC] Ignore public types in GlobalTypeOptimization (WebAssembly#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 WebAssembly#7015
1 parent 6566329 commit 245d915

File tree

2 files changed

+106
-3
lines changed

2 files changed

+106
-3
lines changed

src/passes/GlobalTypeOptimization.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
//
2424

2525
#include "ir/localize.h"
26+
#include "ir/module-utils.h"
2627
#include "ir/ordering.h"
2728
#include "ir/struct-utils.h"
2829
#include "ir/subtypes.h"
@@ -167,13 +168,18 @@ struct GlobalTypeOptimization : public Pass {
167168
auto dataFromSupersMap = std::move(combinedSetGetInfos);
168169
propagator.propagateToSubTypes(dataFromSupersMap);
169170

171+
// Find the public types, which we must not modify.
172+
auto publicTypes = ModuleUtils::getPublicHeapTypes(*module);
173+
std::unordered_set<HeapType> publicTypesSet(publicTypes.begin(),
174+
publicTypes.end());
175+
170176
// Process the propagated info. We look at supertypes first, as the order of
171177
// fields in a supertype is a constraint on what subtypes can do. That is,
172178
// we decide for each supertype what the optimal order is, and consider that
173179
// fixed, and then subtypes can decide how to sort fields that they append.
174180
for (auto type :
175181
HeapTypeOrdering::supertypesFirst(propagator.subTypes.types)) {
176-
if (!type.isStruct()) {
182+
if (!type.isStruct() || publicTypesSet.count(type)) {
177183
continue;
178184
}
179185
auto& fields = type.getStruct().fields;
@@ -291,8 +297,15 @@ struct GlobalTypeOptimization : public Pass {
291297
keptFieldsNotInSuper.push_back(i);
292298
}
293299
} else {
294-
// The super kept this field, so we must keep it as well.
295-
assert(!removableIndexes.count(i));
300+
// The super kept this field, so we must keep it as well. The
301+
// propagation analysis above ensures that we and the super are in
302+
// agreement on keeping it (the reasons that prevent optimization
303+
// propagate to both), except for the corner case of the parent
304+
// being public but us being private (the propagation does not
305+
// take into account visibility).
306+
assert(
307+
!removableIndexes.count(i) ||
308+
(publicTypesSet.count(*super) && !publicTypesSet.count(type)));
296309
// We need to keep it at the same index so we remain compatible.
297310
indexesAfterRemoval[i] = superIndex;
298311
// Update |next| to refer to the next available index. Due to

test/lit/passes/gto-removals.wast

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,3 +1405,93 @@
14051405
)
14061406
)
14071407
)
1408+
1409+
;; Public types cannot be optimized. The function type here is public as the
1410+
;; function is exported, and so the entire rec group is public, and cannot be
1411+
;; modified. We cannot even optimize $child3 which is outside of the rec group,
1412+
;; because its parent is inside. However, we can optimize $unrelated which is
1413+
;; unrelated to them (and so we can remove the field there).
1414+
(module
1415+
(rec
1416+
;; CHECK: (rec
1417+
;; CHECK-NEXT: (type $parent (sub (struct (field (ref func)))))
1418+
(type $parent (sub (struct (field (ref func)))))
1419+
;; CHECK: (type $child1 (sub $parent (struct (field (ref func)))))
1420+
(type $child1 (sub $parent (struct (field (ref func)))))
1421+
;; CHECK: (type $child2 (sub $parent (struct (field (ref func)))))
1422+
(type $child2 (sub $parent (struct (field (ref func)))))
1423+
1424+
;; CHECK: (type $func (func (param (ref $child2))))
1425+
(type $func (func (param $child2 (ref $child2))))
1426+
)
1427+
1428+
;; CHECK: (rec
1429+
;; CHECK-NEXT: (type $unrelated (sub (struct)))
1430+
1431+
;; CHECK: (type $child3 (sub $parent (struct (field (ref func)))))
1432+
(type $child3 (sub $parent (struct (field (ref func)))))
1433+
1434+
(type $unrelated (sub (struct (field (ref func)))))
1435+
1436+
;; CHECK: (elem declare func $func)
1437+
1438+
;; CHECK: (export "func" (func $func))
1439+
1440+
;; CHECK: (func $func (type $func) (param $child2 (ref $child2))
1441+
;; CHECK-NEXT: (drop
1442+
;; CHECK-NEXT: (struct.new $parent
1443+
;; CHECK-NEXT: (ref.func $func)
1444+
;; CHECK-NEXT: )
1445+
;; CHECK-NEXT: )
1446+
;; CHECK-NEXT: (drop
1447+
;; CHECK-NEXT: (struct.new $child1
1448+
;; CHECK-NEXT: (ref.func $func)
1449+
;; CHECK-NEXT: )
1450+
;; CHECK-NEXT: )
1451+
;; CHECK-NEXT: (drop
1452+
;; CHECK-NEXT: (struct.new $child2
1453+
;; CHECK-NEXT: (ref.func $func)
1454+
;; CHECK-NEXT: )
1455+
;; CHECK-NEXT: )
1456+
;; CHECK-NEXT: (drop
1457+
;; CHECK-NEXT: (struct.new $child3
1458+
;; CHECK-NEXT: (ref.func $func)
1459+
;; CHECK-NEXT: )
1460+
;; CHECK-NEXT: )
1461+
;; CHECK-NEXT: (drop
1462+
;; CHECK-NEXT: (struct.new_default $unrelated)
1463+
;; CHECK-NEXT: )
1464+
;; CHECK-NEXT: )
1465+
(func $func (export "func") (type $func) (param $child2 (ref $child2))
1466+
;; Create all the types. Note that the value here is non-nullable, as is the
1467+
;; field, so if we remove the field by mistake in GTO but leave it during
1468+
;; TypeUpdater, we'd error (on providing a default value for a non-nullable
1469+
;; field).
1470+
(drop
1471+
(struct.new $parent
1472+
(ref.func $func)
1473+
)
1474+
)
1475+
(drop
1476+
(struct.new $child1
1477+
(ref.func $func)
1478+
)
1479+
)
1480+
(drop
1481+
(struct.new $child2
1482+
(ref.func $func)
1483+
)
1484+
)
1485+
(drop
1486+
(struct.new $child3
1487+
(ref.func $func)
1488+
)
1489+
)
1490+
;; We can optimize this one, and no other.
1491+
(drop
1492+
(struct.new $unrelated
1493+
(ref.func $func)
1494+
)
1495+
)
1496+
)
1497+
)

0 commit comments

Comments
 (0)