From 245d9156d8bd52ce9bc50c554aa3a9cd42cd0e6b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 17 Oct 2024 14:27:55 -0700 Subject: [PATCH] [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 --- src/passes/GlobalTypeOptimization.cpp | 19 +++++- test/lit/passes/gto-removals.wast | 90 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 3 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index dac6fd7b659..1f0da2595ec 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 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 c2ac66a5793..61396cf8f7a 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) + ) + ) + ) +)