Skip to content

Commit

Permalink
[GC] Ignore public types in GlobalTypeOptimization (#7017)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
kripken authored Oct 17, 2024
1 parent 6566329 commit 245d915
Show file tree
Hide file tree
Showing 2 changed files with 106 additions and 3 deletions.
19 changes: 16 additions & 3 deletions src/passes/GlobalTypeOptimization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down
90 changes: 90 additions & 0 deletions test/lit/passes/gto-removals.wast
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
)
)
)

0 comments on commit 245d915

Please sign in to comment.