From 56d516519b6907973e58680764a608d39a7b22b1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 08:40:23 -0700 Subject: [PATCH 01/57] work --- src/ir/struct-utils.h | 46 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index e1726c05671..20663104b8d 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -44,6 +44,9 @@ template struct StructValues : public std::vector { assert(index < this->size()); return std::vector::operator[](index); } + + // Store the descriptor as another field. + T desc; }; // Maps heap types to a StructValues for that heap type. @@ -80,6 +83,8 @@ struct StructValuesMap : public std::unordered_map> { x.dump(o); o << " "; }; + o << " desc: "; + vec.desc.dump(o); o << '\n'; } } @@ -129,10 +134,18 @@ struct FunctionStructValuesMap // // void noteCopy(HeapType type, Index index, T& info); // -// * Note a read +// * Note a read. // // void noteRead(HeapType type, Index index, T& info); // +// * Note an expression written to the descriptor. +// +// void noteDescExpression(Expression* expr, HeapType type, T& info); +// +// * Note a use of the descriptor. +// +// void noteDescRead(HeapType type, T& info); +// // We track information from struct.new and struct.set/struct.get separately, // because in struct.new we know more about the type - we know the actual exact // type being written to, and not just that it is of a subtype of the @@ -168,6 +181,10 @@ struct StructScanner noteExpressionOrCopy(curr->operands[i], heapType, i, infos[i]); } } + + if (curr->desc) { + noteDescExpression(curr->desc, heapType, infos.desc); + } } void visitStructSet(StructSet* curr) { @@ -236,6 +253,33 @@ struct StructScanner noteExpressionOrCopy(curr->replacement, heapType, index, info); } + void visitRefCast(RefCast* curr) { + if (curr->desc) { + handleDescRead(curr->ref); + } + } + + void visitRefGetDesc(RefGetDesc* curr) { + handleDescRead(curr->ref); + } + + void visitBrOn(BrOn* curr) { + if (curr->desc && (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail)) { + handleDescRead(curr->ref); + } + } + + void handleDescRead(Expression* ref) { + auto type = ref->type; + if (type == Type::unreachable || type.isNull()) { + return; + } + + auto heapType = type.getHeapType(); + self().noteDescRead(heapType, + functionSetGetInfos[this->getFunction()][heapType].desc); + } + void noteExpressionOrCopy(Expression* expr, HeapType type, Index index, T& info) { // Look at the value falling through, if it has the exact same type From 440016f9adb64c9acfc7dbf87b9f301bdf1e3e03 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 08:45:17 -0700 Subject: [PATCH 02/57] work --- src/passes/ConstantFieldPropagation.cpp | 8 ++++++++ src/passes/GlobalTypeOptimization.cpp | 8 ++++++++ src/passes/TypeRefining.cpp | 8 ++++++++ 3 files changed, 24 insertions(+) diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 48e5a9c7770..679e5a80c51 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -462,6 +462,14 @@ struct PCVScanner info.noteUnknown(); } + void noteDescExpression(Expression* expr, HeapType type, PossibleConstantValues& info) { + // TODO + } + + void noteDescRead(HeapType type, PossibleConstantValues& info) { + // TODO + } + BoolFunctionStructValuesMap& functionCopyInfos; }; diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 6c1872d2bb4..32d01d220a5 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -101,6 +101,14 @@ struct FieldInfoScanner info.noteRead(); info.noteWrite(); } + + void noteDescExpression(Expression* expr, HeapType type, FieldInfo& info) { + // TODO + } + + void noteDescRead(HeapType type, FieldInfo& info) { + // TODO + } }; struct GlobalTypeOptimization : public Pass { diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index da113808ec9..8ee820864b8 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -95,6 +95,14 @@ struct FieldInfoScanner info.note(expr->type); } + void noteDescExpression(Expression* expr, HeapType type, FieldInfo& info) { + // TODO + } + + void noteDescRead(HeapType type, FieldInfo& info) { + // TODO + } + Properties::FallthroughBehavior getFallthroughBehavior() { // Looking at fallthrough values may be dangerous here, because it ignores // intermediate steps. Consider this: From bdf10455b6f37d8542a36360b9c9a2bc8815710f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 08:47:00 -0700 Subject: [PATCH 03/57] work --- src/ir/struct-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 20663104b8d..bbca7171445 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -183,7 +183,7 @@ struct StructScanner } if (curr->desc) { - noteDescExpression(curr->desc, heapType, infos.desc); + self().noteDescExpression(curr->desc, heapType, infos.desc); } } From 1e24c1d2214bb926728708f072bbfe8b037324a3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 08:51:58 -0700 Subject: [PATCH 04/57] work --- src/ir/struct-utils.h | 18 ++++++++---------- src/passes/ConstantFieldPropagation.cpp | 8 -------- src/passes/GlobalTypeOptimization.cpp | 8 -------- src/passes/TypeRefining.cpp | 8 -------- 4 files changed, 8 insertions(+), 34 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index bbca7171445..2b0fc54d47a 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -138,18 +138,13 @@ struct FunctionStructValuesMap // // void noteRead(HeapType type, Index index, T& info); // -// * Note an expression written to the descriptor. -// -// void noteDescExpression(Expression* expr, HeapType type, T& info); -// -// * Note a use of the descriptor. -// -// void noteDescRead(HeapType type, T& info); -// // We track information from struct.new and struct.set/struct.get separately, // because in struct.new we know more about the type - we know the actual exact // type being written to, and not just that it is of a subtype of the // instruction's type, which helps later. +// +// Descriptors are treated as fields in that we call the above functions on +// them. We pass DescriptorIndex for their index as a fake value. template struct StructScanner : public WalkerPass>> { @@ -159,6 +154,8 @@ struct StructScanner SubType& self() { return *static_cast(this); } + static const Index DescriptorIndex = -1; + StructScanner(FunctionStructValuesMap& functionNewInfos, FunctionStructValuesMap& functionSetGetInfos) : functionNewInfos(functionNewInfos), @@ -183,7 +180,7 @@ struct StructScanner } if (curr->desc) { - self().noteDescExpression(curr->desc, heapType, infos.desc); + self().noteExpression(curr->desc, heapType, DescriptorIndex, infos.desc); } } @@ -276,7 +273,8 @@ struct StructScanner } auto heapType = type.getHeapType(); - self().noteDescRead(heapType, + self().noteRead(heapType, + DescriptorIndex, functionSetGetInfos[this->getFunction()][heapType].desc); } diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 679e5a80c51..48e5a9c7770 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -462,14 +462,6 @@ struct PCVScanner info.noteUnknown(); } - void noteDescExpression(Expression* expr, HeapType type, PossibleConstantValues& info) { - // TODO - } - - void noteDescRead(HeapType type, PossibleConstantValues& info) { - // TODO - } - BoolFunctionStructValuesMap& functionCopyInfos; }; diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 32d01d220a5..6c1872d2bb4 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -101,14 +101,6 @@ struct FieldInfoScanner info.noteRead(); info.noteWrite(); } - - void noteDescExpression(Expression* expr, HeapType type, FieldInfo& info) { - // TODO - } - - void noteDescRead(HeapType type, FieldInfo& info) { - // TODO - } }; struct GlobalTypeOptimization : public Pass { diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index 8ee820864b8..da113808ec9 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -95,14 +95,6 @@ struct FieldInfoScanner info.note(expr->type); } - void noteDescExpression(Expression* expr, HeapType type, FieldInfo& info) { - // TODO - } - - void noteDescRead(HeapType type, FieldInfo& info) { - // TODO - } - Properties::FallthroughBehavior getFallthroughBehavior() { // Looking at fallthrough values may be dangerous here, because it ignores // intermediate steps. Consider this: From 1e958b5610d96036b20b4a2d624a6f8cee8a693a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 09:03:20 -0700 Subject: [PATCH 05/57] work --- src/passes/GlobalTypeOptimization.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 6c1872d2bb4..276808462b6 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -122,6 +122,9 @@ struct GlobalTypeOptimization : public Pass { static const Index RemovedField = Index(-1); std::unordered_map> indexesAfterRemovals; + // The types that no longer need a descriptor. + std::unordered_set unneededDescriptors; + void run(Module* module) override { if (!module->features.hasGC()) { return; @@ -365,6 +368,17 @@ struct GlobalTypeOptimization : public Pass { indexesAfterRemovals[type] = indexesAfterRemoval; } } + + // Process the descriptor. + if (type.getDescriptor()) { + // Parallel to our handling of field removals, above, but simpler as + // descriptors are immutable and non-optional in struct creation: if we + // and our supers do not read the descriptor, we do not need it. + if (!dataFromSupers.desc.hasRead) { +std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself must no longer describe? + unneededDescriptors.insert(type); + } + } } // If we found fields that can be removed, remove them from instructions. From c3ee74192ef8dc69e3f518c4edcf571ed936a3ee Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 09:03:36 -0700 Subject: [PATCH 06/57] work --- src/passes/GlobalTypeOptimization.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 276808462b6..0be16eb1bab 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -370,7 +370,7 @@ struct GlobalTypeOptimization : public Pass { } // Process the descriptor. - if (type.getDescriptor()) { + if (type.getDescriptorType()) { // Parallel to our handling of field removals, above, but simpler as // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. From 6086dc9b44513b011d8101757222dbf4a3fbb65e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 15:46:26 -0700 Subject: [PATCH 07/57] prep --- test/lit/passes/gto-desc.wast | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index d7762626e35..46e97c5a640 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1,6 +1,6 @@ ;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. -;; RUN: wasm-opt %s -all --closed-world --gto --preserve-type-order -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt -all --closed-world --gto --preserve-type-order -S -o - | filecheck %s (module (rec @@ -128,3 +128,11 @@ ) ) ) + +(module + (rec + (type $A (struct)) + (type $B (descriptor $C (struct))) + (type $C (describes $B (struct))) + ) +) From 7ac2730673723594a5888ef710326cf2d0ff4700 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 15:52:11 -0700 Subject: [PATCH 08/57] work --- test/lit/passes/gto-desc.wast | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 46e97c5a640..dadeba12084 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -129,10 +129,37 @@ ) ) +;; The descriptor here is not needed. (module (rec - (type $A (struct)) - (type $B (descriptor $C (struct))) + (type $A (descriptor $B (struct))) + (type $B (describes $A (struct))) + ) + + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + ;; Even creating the type does not force us to keep the descriptor, if it is + ;; never used. + (drop + (struct.new $A + (struct.new $B) + ) + ) + ) +) + +;; Both descriptors in this chain are unneeded. +(module + (rec + (type $A (descriptor $B (struct))) + (type $B (describes $A (descriptor $C (struct)))) (type $C (describes $B (struct))) ) + + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (local $C (ref $C)) + ) ) From b826d634d47a1377e73d7568ba418233ac1e0dc9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:00:41 -0700 Subject: [PATCH 09/57] work --- src/passes/GlobalTypeOptimization.cpp | 29 +++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 0be16eb1bab..afc472a3d74 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -122,8 +122,10 @@ struct GlobalTypeOptimization : public Pass { static const Index RemovedField = Index(-1); std::unordered_map> indexesAfterRemovals; - // The types that no longer need a descriptor. - std::unordered_set unneededDescriptors; + // The types that no longer need a descriptor, and that no longer need to + // describe. + std::unordered_set haveUnneededDescriptors; + std::unordered_set haveUnneededDescribings; void run(Module* module) override { if (!module->features.hasGC()) { @@ -370,13 +372,14 @@ struct GlobalTypeOptimization : public Pass { } // Process the descriptor. - if (type.getDescriptorType()) { + if (auto desc = type.getDescriptorType()) { // Parallel to our handling of field removals, above, but simpler as // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. if (!dataFromSupers.desc.hasRead) { std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself must no longer describe? - unneededDescriptors.insert(type); + haveUnneededDescriptors.insert(type); + haveUnneededDescribings.insert(*desc); } } } @@ -455,6 +458,24 @@ std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself mus } } } + + void + modifyTypeBuilderEntry(TypeBuilder& typeBuilder, Index i, HeapType oldType) override { + if (!oldType.isStruct()) { + return; + } + + // Remove an unneeded descriptor. + // TODO: check for CD here and above + if (parent.haveUnneededDescriptors.count(oldType)) { + typeBuilder.setDescriptor(i, std::nullopt); + } + + // Remove an unneeded describes. + if (parent.haveUnneededDescribings.count(oldType)) { + typeBuilder.setDescribed(i, std::nullopt); + } + } }; TypeRewriter(wasm, *this).update(); From 427297f82729cbd324e87333d7567ff075e6d157 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:04:18 -0700 Subject: [PATCH 10/57] work --- src/passes/GlobalTypeOptimization.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index afc472a3d74..207879c9895 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -392,8 +392,13 @@ std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself mus removeFieldsInInstructions(*module); } + // Descriptor/describings are in pairs, so the size of these sets is equal, + // and we only need to check one below. + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + // Update the types in the entire module. - if (!indexesAfterRemovals.empty() || !canBecomeImmutable.empty()) { + if (!indexesAfterRemovals.empty() || !canBecomeImmutable.empty() || + !haveUnneededDescriptors.empty()) { updateTypes(*module); } } @@ -461,6 +466,7 @@ std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself mus void modifyTypeBuilderEntry(TypeBuilder& typeBuilder, Index i, HeapType oldType) override { +std::cout << "modTBE " << i << " : " << oldType << '\n'; if (!oldType.isStruct()) { return; } @@ -468,11 +474,13 @@ std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself mus // Remove an unneeded descriptor. // TODO: check for CD here and above if (parent.haveUnneededDescriptors.count(oldType)) { +std::cout << "clear descript\n"; typeBuilder.setDescriptor(i, std::nullopt); } // Remove an unneeded describes. if (parent.haveUnneededDescribings.count(oldType)) { +std::cout << "clear describd\n"; typeBuilder.setDescribed(i, std::nullopt); } } From a2c49145559c05dcc476ffdbd8bad54033a3a260 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:11:33 -0700 Subject: [PATCH 11/57] work --- src/passes/GlobalTypeOptimization.cpp | 67 +++++++++++++++------------ test/lit/passes/gto-desc.wast | 4 ++ 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 207879c9895..66a45667556 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -377,7 +377,6 @@ struct GlobalTypeOptimization : public Pass { // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. if (!dataFromSupers.desc.hasRead) { -std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself must no longer describe? haveUnneededDescriptors.insert(type); haveUnneededDescribings.insert(*desc); } @@ -466,7 +465,6 @@ std::cout << "no need for desc " << type << '\n'; // TODO: desctiptor itself mus void modifyTypeBuilderEntry(TypeBuilder& typeBuilder, Index i, HeapType oldType) override { -std::cout << "modTBE " << i << " : " << oldType << '\n'; if (!oldType.isStruct()) { return; } @@ -474,13 +472,11 @@ std::cout << "modTBE " << i << " : " << oldType << '\n'; // Remove an unneeded descriptor. // TODO: check for CD here and above if (parent.haveUnneededDescriptors.count(oldType)) { -std::cout << "clear descript\n"; typeBuilder.setDescriptor(i, std::nullopt); } // Remove an unneeded describes. if (parent.haveUnneededDescribings.count(oldType)) { -std::cout << "clear describd\n"; typeBuilder.setDescribed(i, std::nullopt); } } @@ -519,14 +515,19 @@ std::cout << "clear describd\n"; return; } - auto iter = parent.indexesAfterRemovals.find(curr->type.getHeapType()); - if (iter == parent.indexesAfterRemovals.end()) { - return; + auto type = curr->type.getHeapType(); + auto removeDesc = parent.haveUnneededDescriptors.count(type); + + // There may be no indexes to remove, if we are only removing the + // descriptor. + std::vector* indexesAfterRemoval = nullptr; + auto iter = parent.indexesAfterRemovals.find(type); + if (iter != parent.indexesAfterRemovals.end()) { + indexesAfterRemoval = &iter->second; } - auto& indexesAfterRemoval = iter->second; auto& operands = curr->operands; - assert(indexesAfterRemoval.size() == operands.size()); + assert(indexesAfterRemoval->size() == operands.size()); // Ensure any children with non-trivial effects are replaced with // local.gets, so that we can remove/reorder to our hearts' content. @@ -541,29 +542,37 @@ std::cout << "clear describd\n"; needEHFixups = true; } - // Remove and reorder operands. - Index removed = 0; - std::vector old(operands.begin(), operands.end()); - for (Index i = 0; i < operands.size(); ++i) { - auto newIndex = indexesAfterRemoval[i]; - if (newIndex != RemovedField) { - assert(newIndex < operands.size()); - operands[newIndex] = old[i]; - } else { - ++removed; - if (!func && - EffectAnalyzer(getPassOptions(), *getModule(), old[i]).trap) { - removedTrappingInits.push_back(old[i]); + if (indexesAfterRemoval) { + // Remove and reorder operands. + Index removed = 0; + std::vector old(operands.begin(), operands.end()); + for (Index i = 0; i < operands.size(); ++i) { + auto newIndex = (*indexesAfterRemoval)[i]; + if (newIndex != RemovedField) { + assert(newIndex < operands.size()); + operands[newIndex] = old[i]; + } else { + ++removed; + if (!func && + EffectAnalyzer(getPassOptions(), *getModule(), old[i]).trap) { + removedTrappingInits.push_back(old[i]); + } } } + if (removed) { + operands.resize(operands.size() - removed); + } else { + // If we didn't remove anything then we must have reordered (or else + // we have done pointless work). + assert(*indexesAfterRemoval != + makeIdentity(indexesAfterRemoval->size())); + } } - if (removed) { - operands.resize(operands.size() - removed); - } else { - // If we didn't remove anything then we must have reordered (or else - // we have done pointless work). - assert(indexesAfterRemoval != - makeIdentity(indexesAfterRemoval.size())); + + // Removing the descriptor is trivial, after the ChildLocalizer we ran + // earlier. + if (removeDesc) { + curr->desc = nullptr; } } diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index dadeba12084..5f81642a885 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -163,3 +163,7 @@ (local $C (ref $C)) ) ) + +;; effects in dropped desc +;; test index removals amd also desc + From 991a129be4c17abb734222435214b17f226451d0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:17:01 -0700 Subject: [PATCH 12/57] work --- src/passes/GlobalTypeOptimization.cpp | 35 +++++++++++++-------------- test/lit/passes/gto-desc.wast | 2 +- 2 files changed, 18 insertions(+), 19 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 66a45667556..bb32ae7db6e 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -383,18 +383,18 @@ struct GlobalTypeOptimization : public Pass { } } - // If we found fields that can be removed, remove them from instructions. + // Descriptor/describings are in pairs, so the size of these sets is equal, + // and we only need to check one below. + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + + // If we found things that can be removed, remove them from instructions. // (Note that we must do this first, while we still have the old heap types // that we can identify, and only after this should we update all the types // throughout the module.) - if (!indexesAfterRemovals.empty()) { - removeFieldsInInstructions(*module); + if (!indexesAfterRemovals.empty() || !haveUnneededDescriptors.empty()) { + updateInstructions(*module); } - // Descriptor/describings are in pairs, so the size of these sets is equal, - // and we only need to check one below. - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); - // Update the types in the entire module. if (!indexesAfterRemovals.empty() || !canBecomeImmutable.empty() || !haveUnneededDescriptors.empty()) { @@ -487,7 +487,7 @@ struct GlobalTypeOptimization : public Pass { // After updating the types to remove certain fields, we must also remove // them from struct instructions. - void removeFieldsInInstructions(Module& wasm) { + void updateInstructions(Module& wasm) { struct FieldRemover : public WalkerPass> { bool isFunctionParallel() override { return true; } @@ -510,10 +510,6 @@ struct GlobalTypeOptimization : public Pass { if (curr->type == Type::unreachable) { return; } - if (curr->isWithDefault()) { - // Nothing to do, a default was written and will no longer be. - return; - } auto type = curr->type.getHeapType(); auto removeDesc = parent.haveUnneededDescriptors.count(type); @@ -521,14 +517,14 @@ struct GlobalTypeOptimization : public Pass { // There may be no indexes to remove, if we are only removing the // descriptor. std::vector* indexesAfterRemoval = nullptr; - auto iter = parent.indexesAfterRemovals.find(type); - if (iter != parent.indexesAfterRemovals.end()) { - indexesAfterRemoval = &iter->second; + // There are also no indexes to remove if we only write default values. + if (!curr->isWithDefault()) { + auto iter = parent.indexesAfterRemovals.find(type); + if (iter != parent.indexesAfterRemovals.end()) { + indexesAfterRemoval = &iter->second; + } } - auto& operands = curr->operands; - assert(indexesAfterRemoval->size() == operands.size()); - // Ensure any children with non-trivial effects are replaced with // local.gets, so that we can remove/reorder to our hearts' content. // We can only do this inside functions. Outside of functions, we @@ -544,6 +540,9 @@ struct GlobalTypeOptimization : public Pass { if (indexesAfterRemoval) { // Remove and reorder operands. + auto& operands = curr->operands; + assert(indexesAfterRemoval->size() == operands.size()); + Index removed = 0; std::vector old(operands.begin(), operands.end()); for (Index i = 0; i < operands.size(); ++i) { diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 5f81642a885..17a0f6797d7 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -166,4 +166,4 @@ ;; effects in dropped desc ;; test index removals amd also desc - +;; test struct new default and not new defualt From b457ed2e0e28d02da7703764d825679f918e941a Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:20:17 -0700 Subject: [PATCH 13/57] work --- test/lit/passes/gto-desc.wast | 67 +++++++++++++++++++++++++++-------- 1 file changed, 53 insertions(+), 14 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 17a0f6797d7..9eb43d8595d 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -5,9 +5,9 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $struct (descriptor $desc (struct (field i32)))) + ;; CHECK-NEXT: (type $struct (struct (field i32))) (type $struct (descriptor $desc (struct (field i32)))) - ;; CHECK: (type $desc (describes $struct (struct))) + ;; CHECK: (type $desc (struct)) (type $desc (describes $struct (struct))) ;; CHECK: (type $pair (struct)) (type $pair (struct (field (ref $struct)) (field (ref $struct)))) @@ -17,6 +17,8 @@ ;; CHECK: (type $4 (func (param (ref $struct) (ref $used-pair)))) + ;; CHECK: (type $5 (func (param (ref $struct)))) + ;; CHECK: (global $nullable-desc (ref null (exact $desc)) (struct.new_default $desc)) (global $nullable-desc (ref null (exact $desc)) (struct.new $desc)) @@ -59,11 +61,9 @@ ;; CHECK: (global $used-traps (ref $used-pair) (struct.new $used-pair ;; CHECK-NEXT: (struct.new $struct ;; CHECK-NEXT: (i32.const 4) - ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (struct.new $struct ;; CHECK-NEXT: (i32.const 5) - ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: ) ;; CHECK-NEXT: )) (global $used-traps (ref $used-pair) @@ -81,16 +81,6 @@ ) ) - ;; CHECK: (global $gto-removed-0 (ref (exact $struct)) (struct.new $struct - ;; CHECK-NEXT: (i32.const 1) - ;; CHECK-NEXT: (ref.null none) - ;; CHECK-NEXT: )) - - ;; CHECK: (global $gto-removed-1_6 (ref (exact $struct)) (struct.new $struct - ;; CHECK-NEXT: (i32.const 2) - ;; CHECK-NEXT: (global.get $nullable-desc) - ;; CHECK-NEXT: )) - ;; CHECK: (func $use-struct-fields (type $4) (param $0 (ref $struct)) (param $1 (ref $used-pair)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (struct.get $struct 0 @@ -127,15 +117,43 @@ ) ) ) + + ;; CHECK: (func $use-desc (type $5) (param $struct (ref $struct)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $struct + ;; CHECK-NEXT: (local.get $struct) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $use-desc (param $struct (ref $struct)) + ;; Use the descriptors, so this test is not trivial. + (drop + (ref.get_desc $struct + (local.get $struct) + ) + ) + ) ) ;; The descriptor here is not needed. (module (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) (type $B (describes $A (struct))) ) + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $test (local $A (ref $A)) (local $B (ref $B)) @@ -152,15 +170,36 @@ ;; Both descriptors in this chain are unneeded. (module (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) (type $B (describes $A (descriptor $C (struct)))) + ;; CHECK: (type $C (struct)) (type $C (describes $B (struct))) ) + ;; CHECK: (type $3 (func)) + + ;; CHECK: (func $test (type $3) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) (func $test (local $A (ref $A)) (local $B (ref $B)) (local $C (ref $C)) + (drop + (struct.new $A + (struct.new $B + (struct.new $C) + ) + ) + ) ) ) From 5b25a96d8a65b107ad55f019ee4622b60e3a5afc Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:23:38 -0700 Subject: [PATCH 14/57] work --- test/lit/passes/gto-desc.wast | 63 +++++++++++++++++++++++++++++++++-- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 9eb43d8595d..36de1729751 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -157,8 +157,32 @@ (func $test (local $A (ref $A)) (local $B (ref $B)) - ;; Even creating the type does not force us to keep the descriptor, if it is - ;; never used. + ) +) + +;; As above, but even creating the type does not force us to keep the +;; descriptor, if it is never used. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) (drop (struct.new $A (struct.new $B) @@ -203,6 +227,41 @@ ) ) +;; ref.get_desc keeps the descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (drop + (struct.new $A + (struct.new $B) + ) + ) + (drop + (ref.get_desc $A + (local.get $A) + ) + ) + ) +) + ;; effects in dropped desc ;; test index removals amd also desc ;; test struct new default and not new defualt From b91aef6481cdc4cb86cc81f29a9a6a96b9941464 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:32:30 -0700 Subject: [PATCH 15/57] work --- src/ir/struct-utils.h | 1 + src/passes/GlobalTypeOptimization.cpp | 2 ++ 2 files changed, 3 insertions(+) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 2b0fc54d47a..972ada4bc25 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -72,6 +72,7 @@ struct StructValuesMap : public std::unordered_map> { for (Index i = 0; i < info.size(); i++) { combinedInfos[type][i].combine(info[i]); } + combinedInfos[type].desc.combine(info.desc); } } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index bb32ae7db6e..47972d6373d 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -94,6 +94,7 @@ struct FieldInfoScanner } void noteRead(HeapType type, Index index, FieldInfo& info) { +std::cout << "note read " << type << " : " << index << '\n'; info.noteRead(); } @@ -373,6 +374,7 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { +std::cout << "type " << type << " with desc " << *desc << " and data from supers " << dataFromSupers.desc.hasRead << '\n'; // Parallel to our handling of field removals, above, but simpler as // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. From a5697f3f94984d5bbfb53e9c47a168eafb83411c Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:33:09 -0700 Subject: [PATCH 16/57] work --- test/lit/passes/gto-desc.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 36de1729751..450b3e6b12f 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -249,7 +249,7 @@ (func $test (local $A (ref $A)) (local $B (ref $B)) - (drop + (local.set $A (struct.new $A (struct.new $B) ) From 48c66cafaf1329ce9acdd1bf6a823b4b6cba16b3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:33:19 -0700 Subject: [PATCH 17/57] work --- test/lit/passes/gto-desc.wast | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 450b3e6b12f..62a9e003579 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -5,9 +5,9 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $struct (struct (field i32))) + ;; CHECK-NEXT: (type $struct (descriptor $desc (struct (field i32)))) (type $struct (descriptor $desc (struct (field i32)))) - ;; CHECK: (type $desc (struct)) + ;; CHECK: (type $desc (describes $struct (struct))) (type $desc (describes $struct (struct))) ;; CHECK: (type $pair (struct)) (type $pair (struct (field (ref $struct)) (field (ref $struct)))) @@ -61,9 +61,11 @@ ;; CHECK: (global $used-traps (ref $used-pair) (struct.new $used-pair ;; CHECK-NEXT: (struct.new $struct ;; CHECK-NEXT: (i32.const 4) + ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (struct.new $struct ;; CHECK-NEXT: (i32.const 5) + ;; CHECK-NEXT: (ref.null none) ;; CHECK-NEXT: ) ;; CHECK-NEXT: )) (global $used-traps (ref $used-pair) @@ -81,6 +83,16 @@ ) ) + ;; CHECK: (global $gto-removed-0 (ref (exact $struct)) (struct.new $struct + ;; CHECK-NEXT: (i32.const 1) + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: )) + + ;; CHECK: (global $gto-removed-1_6 (ref (exact $struct)) (struct.new $struct + ;; CHECK-NEXT: (i32.const 2) + ;; CHECK-NEXT: (global.get $nullable-desc) + ;; CHECK-NEXT: )) + ;; CHECK: (func $use-struct-fields (type $4) (param $0 (ref $struct)) (param $1 (ref $used-pair)) ;; CHECK-NEXT: (drop ;; CHECK-NEXT: (struct.get $struct 0 @@ -150,9 +162,6 @@ ;; CHECK: (func $test (type $2) ;; CHECK-NEXT: (local $A (ref $A)) ;; CHECK-NEXT: (local $B (ref $B)) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.new_default $A) - ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test (local $A (ref $A)) @@ -231,19 +240,26 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct)) + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) (type $A (descriptor $B (struct))) - ;; CHECK: (type $B (struct)) + ;; CHECK: (type $B (describes $A (struct))) (type $B (describes $A (struct))) ) - ;; CHECK: (type $2 (func)) + ;; CHECK: (type $2 (func)) ;; CHECK: (func $test (type $2) ;; CHECK-NEXT: (local $A (ref $A)) ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: (ref.get_desc $A + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test From e002c6190231794ff010ce34b93583f5c414c8c0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:34:49 -0700 Subject: [PATCH 18/57] work --- test/lit/passes/gto-desc.wast | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 62a9e003579..59cc98e63b8 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -138,7 +138,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $use-desc (param $struct (ref $struct)) - ;; Use the descriptors, so this test is not trivial. + ;; Use the descriptor, so this test is not trivial. (drop (ref.get_desc $struct (local.get $struct) @@ -278,6 +278,9 @@ ) ) +;; visitRefCast.desc +;; br on desc + ;; effects in dropped desc ;; test index removals amd also desc ;; test struct new default and not new defualt From 5be8c2192d0ddcd4e622f8b5375457c5043b2de2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:48:00 -0700 Subject: [PATCH 19/57] work --- test/lit/passes/gto-desc.wast | 46 +++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 59cc98e63b8..79cedd84e08 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -278,9 +278,55 @@ ) ) +;; ref.cast_desc keeps the descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (describes $A (struct))) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.cast_desc (ref (exact $A)) + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (local.set $A + (struct.new $A + (struct.new $B) + ) + ) + (drop + (ref.cast_desc (ref null (exact $A)) + (local.get $A) + (struct.new $B) + ) + ) + ) +) + ;; visitRefCast.desc ;; br on desc ;; effects in dropped desc ;; test index removals amd also desc ;; test struct new default and not new defualt + +;; test multipledescriptors in one module, only one removfd From 33e9e8792fe0545e7c8c7b4a216d96b1c51892a1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:50:07 -0700 Subject: [PATCH 20/57] work --- test/lit/passes/gto-desc.wast | 105 +++++++++++++++++++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 79cedd84e08..c90a471b8e6 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -322,8 +322,109 @@ ) ) -;; visitRefCast.desc -;; br on desc +;; br_on_cast_desc keeps the descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (describes $A (struct))) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block $l (result (ref null $A)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (br_on_cast_desc $l (ref $A) (ref (exact $A)) + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (local.set $A + (struct.new $A + (struct.new $B) + ) + ) + (drop + (block $l (result (ref null $A)) + (br_on_cast_desc $l anyref (ref null $A) + (local.get $A) + (struct.new $B) + ) + (unreachable) + ) + ) + ) +) + +;; br_on_cast_desc_fail keeps the descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (describes $A (struct))) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block $l (result (ref null $A)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (br_on_cast_desc_fail $l (ref $A) (ref (exact $A)) + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (unreachable) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (local.set $A + (struct.new $A + (struct.new $B) + ) + ) + (drop + (block $l (result (ref null $A)) + (br_on_cast_desc_fail $l anyref (ref null $A) + (local.get $A) + (struct.new $B) + ) + (unreachable) + ) + ) + ) +) ;; effects in dropped desc ;; test index removals amd also desc From 3ed905ab222a28096e95cfc33842cf2d20927300 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:51:25 -0700 Subject: [PATCH 21/57] work --- test/lit/passes/gto-desc.wast | 43 ++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index c90a471b8e6..19e1104186c 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -4,6 +4,8 @@ (module (rec + ;; CHECK: (type $struct.0 (struct (field (ref $struct.1)) (field (ref $struct.1)))) + ;; CHECK: (rec ;; CHECK-NEXT: (type $struct (descriptor $desc (struct (field i32)))) (type $struct (descriptor $desc (struct (field i32)))) @@ -426,7 +428,46 @@ ) ) -;; effects in dropped desc +;; The descriptor can be removed, but its effects must be preserved. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref (exact $B))) + ;; CHECK-NEXT: (local $2 (ref (exact $B))) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (block (result (ref (exact $A))) + ;; CHECK-NEXT: (local.set $2 + ;; CHECK-NEXT: (local.tee $B + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref (exact $B))) + (drop + (struct.new $A + (local.tee $B + (struct.new $B) + ) + ) + ) + ) +) + ;; test index removals amd also desc ;; test struct new default and not new defualt From 7b205cc061105b0840deb69e79bc5a1356e0f373 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:57:20 -0700 Subject: [PATCH 22/57] work --- test/lit/passes/gto-desc.wast | 89 ++++++++++++++++++++++++++++++++++- 1 file changed, 88 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 19e1104186c..138fafeca50 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -468,7 +468,94 @@ ) ) -;; test index removals amd also desc +;; Test removing indexes /immutability as well as removing a descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct (field i32))) + (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct (field (mut i32))))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref (exact $B))) + ;; CHECK-NEXT: (local $2 (ref (exact $B))) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (block (result (ref (exact $A))) + ;; CHECK-NEXT: (local.set $2 + ;; CHECK-NEXT: (local.tee $B + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.new $A + ;; CHECK-NEXT: (i32.const 30) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (block (result (ref $A)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.get $A 0 + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (block (result (ref (exact $B))) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 9999) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref (exact $B))) + (local.set $A + (struct.new $A + (i32.const 10) + (i32.const 20) + (i32.const 30) + (local.tee $B + (struct.new $B + (i32.const 1337) + ) + ) + ) + ) + ;; Write only the middle field (we can remove it as write-only). + (struct.set $A 1 + (local.get $A) + (i32.const 42) + ) + ;; Read only the last field. We can make it immutable. + (drop + (struct.get $A 2 + (local.get $A) + ) + ) + ;; Also mutate the field in the descriptor, which should not bother us. + (struct.set $B 0 + (local.get $B) + (i32.const 9999) + ) + ) +) + ;; test struct new default and not new defualt ;; test multipledescriptors in one module, only one removfd From b62a6ecdffe4a5cb8ae8748cfe8b81afb6fca827 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 16:58:00 -0700 Subject: [PATCH 23/57] work --- test/lit/passes/gto-desc.wast | 82 ++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 138fafeca50..d18076f1960 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -4,8 +4,6 @@ (module (rec - ;; CHECK: (type $struct.0 (struct (field (ref $struct.1)) (field (ref $struct.1)))) - ;; CHECK: (rec ;; CHECK-NEXT: (type $struct (descriptor $desc (struct (field i32)))) (type $struct (descriptor $desc (struct (field i32)))) @@ -556,6 +554,86 @@ ) ) +;; As above, but with struct.new_default. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct (field i32))) + (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct (field (mut i32))))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref (exact $B))) + ;; CHECK-NEXT: (local $2 (ref (exact $B))) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (block (result (ref (exact $A))) + ;; CHECK-NEXT: (local.set $2 + ;; CHECK-NEXT: (local.tee $B + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (block (result (ref $A)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 42) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.get $A 0 + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.as_non_null + ;; CHECK-NEXT: (block (result (ref (exact $B))) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (i32.const 9999) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.get $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref (exact $B))) + (local.set $A + (struct.new_default $A + (local.tee $B + (struct.new $B + (i32.const 1337) + ) + ) + ) + ) + (struct.set $A 1 + (local.get $A) + (i32.const 42) + ) + (drop + (struct.get $A 2 + (local.get $A) + ) + ) + (struct.set $B 0 + (local.get $B) + (i32.const 9999) + ) + ) +) + ;; test struct new default and not new defualt ;; test multipledescriptors in one module, only one removfd From 98aa91345a19fd6f8cf0e06f55b6ce27df8946d2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Thu, 28 Aug 2025 17:00:07 -0700 Subject: [PATCH 24/57] work --- test/lit/passes/gto-desc.wast | 59 +++++++++++++++++++++++++++++++++-- 1 file changed, 57 insertions(+), 2 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index d18076f1960..04bf534a0d2 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -634,6 +634,61 @@ ) ) -;; test struct new default and not new defualt +;; Multiple types with descriptors, only one of whom can be removed. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + + ;; CHECK: (type $C (descriptor $D (struct))) + (type $C (descriptor $D (struct))) + ;; CHECK: (type $D (describes $C (struct))) + (type $D (describes $C (struct))) + ) + + ;; CHECK: (type $4 (func)) + + ;; CHECK: (func $test (type $4) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $C + ;; CHECK-NEXT: (struct.new_default $C + ;; CHECK-NEXT: (struct.new_default $D) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $C + ;; CHECK-NEXT: (local.get $C) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $C (ref $C)) + (local.set $A + (struct.new $A + (struct.new $B) + ) + ) + (local.set $C + (struct.new $C + (struct.new $D) + ) + ) + ;; Use the descriptor in $C but not $A. + (drop + (ref.get_desc $C + (local.get $C) + ) + ) + ) +) + +;; subtyping -;; test multipledescriptors in one module, only one removfd From c6f45fea4b9b3c58b731b88c77b11678424d7a1d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 07:00:40 -0700 Subject: [PATCH 25/57] work --- test/lit/passes/gto-desc.wast | 64 ++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 04bf534a0d2..7541c15e3bb 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -690,5 +690,67 @@ ) ) -;; subtyping +;; Subtyping. All these descriptors can be optimized away. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (struct))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (struct))) + (type $A.desc (sub (describes $A (struct)))) + + ;; CHECK: (type $B (sub $A (struct))) + (type $B (sub $A (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (struct))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + + ;; CHECK: (type $C (sub $B (struct))) + (type $C (sub $B (descriptor $C.desc (struct)))) + ;; CHECK: (type $C.desc (sub $B.desc (struct))) + (type $C.desc (sub $B.desc (describes $C (struct)))) + ) + + ;; CHECK: (type $6 (func)) + + ;; CHECK: (func $test (type $6) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $A.desc (ref $A.desc)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $B.desc (ref $B.desc)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (local $C.desc (ref $C.desc)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $B + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $C + ;; CHECK-NEXT: (struct.new_default $C) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $A.desc (ref $A.desc)) + (local $B (ref $B)) + (local $B.desc (ref $B.desc)) + (local $C (ref $C)) + (local $C.desc (ref $C.desc)) + (local.set $A + (struct.new $A + (struct.new $A.desc) + ) + ) + (local.set $B + (struct.new $B + (struct.new $B.desc) + ) + ) + (local.set $C + (struct.new $C + (struct.new $C.desc) + ) + ) + ) +) From 4f05e6f07c2895fe7cae1621bf8b5042f69c89c4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 07:14:58 -0700 Subject: [PATCH 26/57] work --- src/passes/GlobalTypeOptimization.cpp | 42 +++++++++++++--- test/lit/passes/gto-desc.wast | 70 +++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 6 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 47972d6373d..9d27c7fafb4 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -94,7 +94,6 @@ struct FieldInfoScanner } void noteRead(HeapType type, Index index, FieldInfo& info) { -std::cout << "note read " << type << " : " << index << '\n'; info.noteRead(); } @@ -374,20 +373,51 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { -std::cout << "type " << type << " with desc " << *desc << " and data from supers " << dataFromSupers.desc.hasRead << '\n'; // Parallel to our handling of field removals, above, but simpler as // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. + // + // This is only the initial processing, see below. if (!dataFromSupers.desc.hasRead) { haveUnneededDescriptors.insert(type); - haveUnneededDescribings.insert(*desc); } } } - // Descriptor/describings are in pairs, so the size of these sets is equal, - // and we only need to check one below. - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + // After finding unneeded descriptors, apply the subtyping rule that a + // descriptor's subtypes and supertypes must also have descriptors. That + // means we can only remove entire subtyping chains at once, not parts of + // them. + if (!haveUnneededDescriptors.empty()) { + SubTypes subTypes(*module); + // TODO: but not quadratic + std::unordered_set haveUnneededDescriptorsCopy = std::move(haveUnneededDescriptors); + for (auto type : haveUnneededDescriptorsCopy) { + // Check the super. + if (auto super = type.getSuperType()) { + if (!haveUnneededDescriptorsCopy.count(*super)) { + continue; + } + } + // Check the subs. + for (auto sub : subTypes.getImmediateSubTypes(type)) { + if (!haveUnneededDescriptorsCopy.count(sub)) { + continue; + } + } + // No problems: Optimize. + haveUnneededDescriptors.insert(type); + } + + // Now that we know which we can optimize, mark their paired elements for + // the processing below. + for (auto type : haveUnneededDescriptors) { + auto desc = type.getDescriptorType(); + assert(desc); + haveUnneededDescribings.insert(*desc); + } + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + } // If we found things that can be removed, remove them from instructions. // (Note that we must do this first, while we still have the old heap types diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 7541c15e3bb..b2f007df283 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -754,3 +754,73 @@ ) ) +;; As above, but add a use of $C's descriptor. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (struct))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (struct))) + (type $A.desc (sub (describes $A (struct)))) + + ;; CHECK: (type $B (sub $A (struct))) + (type $B (sub $A (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (struct))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + + ;; CHECK: (type $C (sub $B (struct))) + (type $C (sub $B (descriptor $C.desc (struct)))) + ;; CHECK: (type $C.desc (sub $B.desc (struct))) + (type $C.desc (sub $B.desc (describes $C (struct)))) + ) + + ;; CHECK: (type $6 (func)) + + ;; CHECK: (func $test (type $6) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $A.desc (ref $A.desc)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $B.desc (ref $B.desc)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (local $C.desc (ref $C.desc)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $B + ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $C + ;; CHECK-NEXT: (struct.new_default $C) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $A.desc (ref $A.desc)) + (local $B (ref $B)) + (local $B.desc (ref $B.desc)) + (local $C (ref $C)) + (local $C.desc (ref $C.desc)) + (local.set $A + (struct.new $A + (struct.new $A.desc) + ) + ) + (local.set $B + (struct.new $B + (struct.new $B.desc) + ) + ) + (local.set $C + (struct.new $C + (struct.new $C.desc) + ) + ) + ;; This is new. + (drop + (ref.get_desc $C + (local.get $C) + ) + ) + ) +) + From 1e4d63f4efbb6fa0b3e40074cb63e3733bdacfcb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 07:52:44 -0700 Subject: [PATCH 27/57] work --- src/ir/struct-utils.h | 8 ++ src/passes/GlobalTypeOptimization.cpp | 43 +----- test/lit/passes/gto-desc.wast | 196 ++++++++++++++++++++++++-- 3 files changed, 198 insertions(+), 49 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 972ada4bc25..ecd80976867 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -363,6 +363,10 @@ template class TypeHierarchyPropagator { work.push(*superType); } } + // Propagate the descriptor. + if (superInfos.desc.combine(infos.desc)) { + work.push(*superType); + } } } @@ -376,6 +380,10 @@ template class TypeHierarchyPropagator { work.push(subType); } } + // Propagate the descriptor. + if (subInfos.desc.combine(infos.desc)) { + work.push(subType); + } } } } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 9d27c7fafb4..7a11a5883e1 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -28,7 +28,6 @@ #include "ir/names.h" #include "ir/ordering.h" #include "ir/struct-utils.h" -#include "ir/subtypes.h" #include "ir/type-updating.h" #include "ir/utils.h" #include "pass.h" @@ -376,48 +375,16 @@ struct GlobalTypeOptimization : public Pass { // Parallel to our handling of field removals, above, but simpler as // descriptors are immutable and non-optional in struct creation: if we // and our supers do not read the descriptor, we do not need it. - // - // This is only the initial processing, see below. - if (!dataFromSupers.desc.hasRead) { + if (!dataFromSubsAndSupers.desc.hasRead) { haveUnneededDescriptors.insert(type); + haveUnneededDescribings.insert(*desc); } } } - // After finding unneeded descriptors, apply the subtyping rule that a - // descriptor's subtypes and supertypes must also have descriptors. That - // means we can only remove entire subtyping chains at once, not parts of - // them. - if (!haveUnneededDescriptors.empty()) { - SubTypes subTypes(*module); - // TODO: but not quadratic - std::unordered_set haveUnneededDescriptorsCopy = std::move(haveUnneededDescriptors); - for (auto type : haveUnneededDescriptorsCopy) { - // Check the super. - if (auto super = type.getSuperType()) { - if (!haveUnneededDescriptorsCopy.count(*super)) { - continue; - } - } - // Check the subs. - for (auto sub : subTypes.getImmediateSubTypes(type)) { - if (!haveUnneededDescriptorsCopy.count(sub)) { - continue; - } - } - // No problems: Optimize. - haveUnneededDescriptors.insert(type); - } - - // Now that we know which we can optimize, mark their paired elements for - // the processing below. - for (auto type : haveUnneededDescriptors) { - auto desc = type.getDescriptorType(); - assert(desc); - haveUnneededDescribings.insert(*desc); - } - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); - } + // Descriptor/describings are in pairs, so the size of these sets is equal, + // and we only need to check one below. + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); // If we found things that can be removed, remove them from instructions. // (Note that we must do this first, while we still have the old heap types diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index b2f007df283..2f4e236259b 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -754,27 +754,28 @@ ) ) -;; As above, but add a use of $C's descriptor. +;; As above, but add a use of $A's descriptor. We cannot remove a descriptor +;; without removing it from subtypes, so we cannot optimize anything. (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (sub (struct))) + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) (type $A (sub (descriptor $A.desc (struct)))) - ;; CHECK: (type $A.desc (sub (struct))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) (type $A.desc (sub (describes $A (struct)))) - ;; CHECK: (type $B (sub $A (struct))) + ;; CHECK: (type $B (sub $A (descriptor $B.desc (struct)))) (type $B (sub $A (descriptor $B.desc (struct)))) - ;; CHECK: (type $B.desc (sub $A.desc (struct))) + ;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct)))) (type $B.desc (sub $A.desc (describes $B (struct)))) - ;; CHECK: (type $C (sub $B (struct))) + ;; CHECK: (type $C (sub $B (descriptor $C.desc (struct)))) (type $C (sub $B (descriptor $C.desc (struct)))) - ;; CHECK: (type $C.desc (sub $B.desc (struct))) + ;; CHECK: (type $C.desc (sub $B.desc (describes $C (struct)))) (type $C.desc (sub $B.desc (describes $C (struct)))) ) - ;; CHECK: (type $6 (func)) + ;; CHECK: (type $6 (func)) ;; CHECK: (func $test (type $6) ;; CHECK-NEXT: (local $A (ref $A)) @@ -784,13 +785,24 @@ ;; CHECK-NEXT: (local $C (ref $C)) ;; CHECK-NEXT: (local $C.desc (ref $C.desc)) ;; CHECK-NEXT: (local.set $A - ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $A.desc) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $B - ;; CHECK-NEXT: (struct.new_default $B) + ;; CHECK-NEXT: (struct.new_default $B + ;; CHECK-NEXT: (struct.new_default $B.desc) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (local.set $C - ;; CHECK-NEXT: (struct.new_default $C) + ;; CHECK-NEXT: (struct.new_default $C + ;; CHECK-NEXT: (struct.new_default $C.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $A + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) (func $test @@ -816,6 +828,168 @@ ) ) ;; This is new. + (drop + (ref.get_desc $A + (local.get $A) + ) + ) + ) +) + +;; As above, but use $B's. This also stops everything. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + (type $A.desc (sub (describes $A (struct)))) + + ;; CHECK: (type $B (sub $A (descriptor $B.desc (struct)))) + (type $B (sub $A (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct)))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + + ;; CHECK: (type $C (sub $B (descriptor $C.desc (struct)))) + (type $C (sub $B (descriptor $C.desc (struct)))) + ;; CHECK: (type $C.desc (sub $B.desc (describes $C (struct)))) + (type $C.desc (sub $B.desc (describes $C (struct)))) + ) + + ;; CHECK: (type $6 (func)) + + ;; CHECK: (func $test (type $6) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $A.desc (ref $A.desc)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $B.desc (ref $B.desc)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (local $C.desc (ref $C.desc)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $A.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $B + ;; CHECK-NEXT: (struct.new_default $B + ;; CHECK-NEXT: (struct.new_default $B.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $C + ;; CHECK-NEXT: (struct.new_default $C + ;; CHECK-NEXT: (struct.new_default $C.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $B + ;; CHECK-NEXT: (local.get $B) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $A.desc (ref $A.desc)) + (local $B (ref $B)) + (local $B.desc (ref $B.desc)) + (local $C (ref $C)) + (local $C.desc (ref $C.desc)) + (local.set $A + (struct.new $A + (struct.new $A.desc) + ) + ) + (local.set $B + (struct.new $B + (struct.new $B.desc) + ) + ) + (local.set $C + (struct.new $C + (struct.new $C.desc) + ) + ) + ;; This changed. + (drop + (ref.get_desc $B + (local.get $B) + ) + ) + ) +) + +;; As above, with $C. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + (type $A.desc (sub (describes $A (struct)))) + + ;; CHECK: (type $B (sub $A (descriptor $B.desc (struct)))) + (type $B (sub $A (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct)))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + + ;; CHECK: (type $C (sub $B (descriptor $C.desc (struct)))) + (type $C (sub $B (descriptor $C.desc (struct)))) + ;; CHECK: (type $C.desc (sub $B.desc (describes $C (struct)))) + (type $C.desc (sub $B.desc (describes $C (struct)))) + ) + + ;; CHECK: (type $6 (func)) + + ;; CHECK: (func $test (type $6) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $A.desc (ref $A.desc)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $B.desc (ref $B.desc)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: (local $C.desc (ref $C.desc)) + ;; CHECK-NEXT: (local.set $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $A.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $B + ;; CHECK-NEXT: (struct.new_default $B + ;; CHECK-NEXT: (struct.new_default $B.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (local.set $C + ;; CHECK-NEXT: (struct.new_default $C + ;; CHECK-NEXT: (struct.new_default $C.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $C + ;; CHECK-NEXT: (local.get $C) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test + (local $A (ref $A)) + (local $A.desc (ref $A.desc)) + (local $B (ref $B)) + (local $B.desc (ref $B.desc)) + (local $C (ref $C)) + (local $C.desc (ref $C.desc)) + (local.set $A + (struct.new $A + (struct.new $A.desc) + ) + ) + (local.set $B + (struct.new $B + (struct.new $B.desc) + ) + ) + (local.set $C + (struct.new $C + (struct.new $C.desc) + ) + ) + ;; This changed. (drop (ref.get_desc $C (local.get $C) From 757e50bd4cd29e24b298ce1a5bd7cab7e93e15c3 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 08:37:56 -0700 Subject: [PATCH 28/57] work --- src/passes/GlobalTypeOptimization.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 7a11a5883e1..b379b9c4627 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -372,9 +372,8 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { - // Parallel to our handling of field removals, above, but simpler as - // descriptors are immutable and non-optional in struct creation: if we - // and our supers do not read the descriptor, we do not need it. + // To remove a descriptor, it must not be used in either subtypes or + // supertypes, to not break validation. if (!dataFromSubsAndSupers.desc.hasRead) { haveUnneededDescriptors.insert(type); haveUnneededDescribings.insert(*desc); From 42f00fcbb6a6a73d47824311d39892a061533f3e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 08:45:35 -0700 Subject: [PATCH 29/57] work --- src/passes/ConstantFieldPropagation.cpp | 4 ++++ src/passes/TypeRefining.cpp | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index 48e5a9c7770..b76d631ed83 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -443,6 +443,10 @@ struct PCVScanner } void noteCopy(HeapType type, Index index, PossibleConstantValues& info) { + if (index == DescriptorIndex) { + return; + } + // Note copies, as they must be considered later. See the comment on the // propagation of values below. functionCopyInfos[getFunction()][type][index] = true; diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index da113808ec9..49ced6d2038 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -61,6 +61,10 @@ struct FieldInfoScanner HeapType type, Index index, FieldInfo& info) { + if (index == DescriptorIndex) { + return; + } + auto noted = expr->type; // Do not introduce new exact fields that might requires invalid // casts. Keep any existing exact fields, though. From 90751899e9cfbe6067a45e427344f38cf6e782ce Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 08:46:43 -0700 Subject: [PATCH 30/57] format --- src/ir/struct-utils.h | 7 +++---- src/passes/GlobalTypeOptimization.cpp | 5 +++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index ecd80976867..f208d624893 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -257,12 +257,11 @@ struct StructScanner } } - void visitRefGetDesc(RefGetDesc* curr) { - handleDescRead(curr->ref); - } + void visitRefGetDesc(RefGetDesc* curr) { handleDescRead(curr->ref); } void visitBrOn(BrOn* curr) { - if (curr->desc && (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail)) { + if (curr->desc && + (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail)) { handleDescRead(curr->ref); } } diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index b379b9c4627..674523c338c 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -461,8 +461,9 @@ struct GlobalTypeOptimization : public Pass { } } - void - modifyTypeBuilderEntry(TypeBuilder& typeBuilder, Index i, HeapType oldType) override { + void modifyTypeBuilderEntry(TypeBuilder& typeBuilder, + Index i, + HeapType oldType) override { if (!oldType.isStruct()) { return; } From cf29d902ce288b5b8b6e0c03031b224c04c983bd Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 08:56:30 -0700 Subject: [PATCH 31/57] work --- src/passes/GlobalTypeOptimization.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 674523c338c..550ae9b8044 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -469,7 +469,6 @@ struct GlobalTypeOptimization : public Pass { } // Remove an unneeded descriptor. - // TODO: check for CD here and above if (parent.haveUnneededDescriptors.count(oldType)) { typeBuilder.setDescriptor(i, std::nullopt); } From 411650188a3b08f585c6d4b60876d1594e310c28 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 09:22:38 -0700 Subject: [PATCH 32/57] work --- src/passes/GlobalTypeOptimization.cpp | 24 ++++++++++++++++++++---- test/lit/passes/gto-desc.wast | 23 +++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 550ae9b8044..3863e26bbc1 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -80,6 +80,17 @@ struct FieldInfoScanner HeapType type, Index index, FieldInfo& info) { + if (index == DescriptorIndex) { + // Descriptors are immutable, so normal writes do not concern us. However, + // we need to handle the case of a trapping null descriptor, which means + // we cannot remove a descriptor as easily as a normal field. To handle + // that, mark a descriptor as written if we write a nullable value to it, + // basically repurposing the |hasWrite| field to track this effect, in the + // sense of "has a dangerous (trappable) write". + if (!expr->type.isNullable()) { + return; + } + } info.noteWrite(); } @@ -135,6 +146,8 @@ struct GlobalTypeOptimization : public Pass { Fatal() << "GTO requires --closed-world"; } + auto trapsNeverHappen = getPassOptions().trapsNeverHappen; + // Find and analyze struct operations inside each function. StructUtils::FunctionStructValuesMap functionNewInfos(*module), functionSetGetInfos(*module); @@ -373,8 +386,13 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { // To remove a descriptor, it must not be used in either subtypes or - // supertypes, to not break validation. - if (!dataFromSubsAndSupers.desc.hasRead) { + // supertypes, to not break validation. It must also have no write (see + // above, we note only dangerous writes which might trap), as if it + // could trap, we'd have no easy way to remove it in a global scope. + // TODO: We could check and handle the global scope specifically, but + // the trapsNeverHappen flag avoids this problem entirely anyhow. + if (!dataFromSubsAndSupers.desc.hasRead && + (!dataFromSubsAndSupers.desc.hasWrite || trapsNeverHappen)) { haveUnneededDescriptors.insert(type); haveUnneededDescribings.insert(*desc); } @@ -566,8 +584,6 @@ struct GlobalTypeOptimization : public Pass { } } - // Removing the descriptor is trivial, after the ChildLocalizer we ran - // earlier. if (removeDesc) { curr->desc = nullptr; } diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 2f4e236259b..4af4adb4617 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -200,6 +200,29 @@ ) ) +;; As above, with the creation in the global scope. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + + ;; CHECK: (global $g anyref (struct.new_default $A)) + (global $g anyref (struct.new $A + (struct.new $B) + ) + )) + + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + ) +) + ;; Both descriptors in this chain are unneeded. (module (rec From 62ef969fbd155edf34dbbfceac51bd9facfbbb7e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 10:09:45 -0700 Subject: [PATCH 33/57] work --- src/passes/GlobalTypeOptimization.cpp | 30 ++++++++----- test/lit/passes/gto-desc-tnh.wast | 65 +++++++++++++++++++++++++++ test/lit/passes/gto-desc.wast | 11 +++-- 3 files changed, 93 insertions(+), 13 deletions(-) create mode 100644 test/lit/passes/gto-desc-tnh.wast diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 3863e26bbc1..25d30d6e3a5 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -43,6 +43,10 @@ namespace { // Information about usage of a field. struct FieldInfo { + // This represents a normal write for normal fields. For a descriptor, we only + // note "dangerous" writes, specifically ones which might trap (when the + // descriptor in a struct.new is nullable), which is a special situation we + // must avoid. bool hasWrite = false; bool hasRead = false; @@ -61,6 +65,10 @@ struct FieldInfo { } return changed; } + + void dump(std::ostream& o) { + o << "[write: " << hasWrite << " hasRead: " << hasRead << ']'; + } }; struct FieldInfoScanner @@ -81,12 +89,6 @@ struct FieldInfoScanner Index index, FieldInfo& info) { if (index == DescriptorIndex) { - // Descriptors are immutable, so normal writes do not concern us. However, - // we need to handle the case of a trapping null descriptor, which means - // we cannot remove a descriptor as easily as a normal field. To handle - // that, mark a descriptor as written if we write a nullable value to it, - // basically repurposing the |hasWrite| field to track this effect, in the - // sense of "has a dangerous (trappable) write". if (!expr->type.isNullable()) { return; } @@ -146,8 +148,6 @@ struct GlobalTypeOptimization : public Pass { Fatal() << "GTO requires --closed-world"; } - auto trapsNeverHappen = getPassOptions().trapsNeverHappen; - // Find and analyze struct operations inside each function. StructUtils::FunctionStructValuesMap functionNewInfos(*module), functionSetGetInfos(*module); @@ -157,8 +157,14 @@ struct GlobalTypeOptimization : public Pass { // Combine the data from the functions. functionSetGetInfos.combineInto(combinedSetGetInfos); - // TODO: combine newInfos as well, once we have a need for that (we will - // when we do things like subtyping). + + // We need info from struct.news in only one situation, so far: custom + // descriptors, where setting a nullable descriptor is a dangerous write, + // one with effects, that we must track. (Otherwise, nothing in a struct.new + // can prevent removing fields or making them immutable.) + if (module->features.hasCustomDescriptors()) { + functionNewInfos.combineInto(combinedSetGetInfos); + } // Propagate information to super and subtypes on set/get infos: // @@ -197,6 +203,8 @@ struct GlobalTypeOptimization : public Pass { std::unordered_set publicTypesSet(publicTypes.begin(), publicTypes.end()); + auto trapsNeverHappen = getPassOptions().trapsNeverHappen; + // 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 @@ -585,6 +593,8 @@ struct GlobalTypeOptimization : public Pass { } if (removeDesc) { + // We already handled the case of a possible trap here, so we can just + // remove the descriptor. curr->desc = nullptr; } } diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast new file mode 100644 index 00000000000..9bc7bd4a395 --- /dev/null +++ b/test/lit/passes/gto-desc-tnh.wast @@ -0,0 +1,65 @@ +;; NOTE: Assertions have been generated by update_lit_checks.py --all-items and should not be edited. + +;; RUN: foreach %s %t wasm-opt -all --closed-world --gto --preserve-type-order -S -o - | filecheck %s +;; RUN: foreach %s %t wasm-opt -tnh -all --closed-world --gto --preserve-type-order -S -o - | filecheck %s --check-prefix=T_N_H + +;; The descriptor passed in is nullable, so it might trap. We only remove it +;; when traps never happen. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (param $nullable (ref null $B)) + (local $A (ref $A)) + (local $B (ref $B)) + (drop + (struct.new $A + (local.get $nullable) + ) + ) + ) +) + +;; As above, with the struct.new in the global scope. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (struct)) + (type $B (describes $A (struct))) + ) + + ;; CHECK: (type $2 (func)) + + (global $g anyref (struct.new $A + (struct.new $B) + )) + + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (param $nullable (ref null $B)) + (local $A (ref $A)) + (local $B (ref $B)) + ) +) + diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 4af4adb4617..65f023b91ec 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -211,12 +211,17 @@ ) + ;; CHECK: (type $2 (func)) + ;; CHECK: (global $g anyref (struct.new_default $A)) (global $g anyref (struct.new $A (struct.new $B) - ) )) + ;; CHECK: (func $test (type $2) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: ) (func $test (local $A (ref $A)) (local $B (ref $B)) @@ -493,7 +498,7 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct (field i32))) + ;; CHECK-NEXT: (type $A (struct (field (mut i32)))) (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) ;; CHECK: (type $B (struct)) (type $B (describes $A (struct (field (mut i32))))) @@ -581,7 +586,7 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct (field i32))) + ;; CHECK-NEXT: (type $A (struct (field (mut i32)))) (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) ;; CHECK: (type $B (struct)) (type $B (describes $A (struct (field (mut i32))))) From bd5d8fcb37fbc78439c4f912ae8d4a31bd2aa6c5 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 10:11:52 -0700 Subject: [PATCH 34/57] test --- test/lit/passes/gto-desc-tnh.wast | 53 +++++++++++++++++++++++-------- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index 9bc7bd4a395..e221d4e9047 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -8,22 +8,36 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct)) + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) + ;; T_N_H: (rec + ;; T_N_H-NEXT: (type $A (struct)) (type $A (descriptor $B (struct))) - ;; CHECK: (type $B (struct)) + ;; CHECK: (type $B (describes $A (struct))) + ;; T_N_H: (type $B (struct)) (type $B (describes $A (struct))) ) - ;; CHECK: (type $2 (func)) + ;; CHECK: (type $2 (func (param (ref null (exact $B))))) - ;; CHECK: (func $test (type $2) + ;; CHECK: (func $test (type $2) (param $nullable (ref null (exact $B))) ;; CHECK-NEXT: (local $A (ref $A)) ;; CHECK-NEXT: (local $B (ref $B)) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.new_default $A) + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (local.get $nullable) + ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $test (param $nullable (ref null $B)) + ;; T_N_H: (type $2 (func (param (ref null (exact $B))))) + + ;; T_N_H: (func $test (type $2) (param $nullable (ref null (exact $B))) + ;; T_N_H-NEXT: (local $A (ref $A)) + ;; T_N_H-NEXT: (local $B (ref $B)) + ;; T_N_H-NEXT: (drop + ;; T_N_H-NEXT: (struct.new_default $A) + ;; T_N_H-NEXT: ) + ;; T_N_H-NEXT: ) + (func $test (param $nullable (ref null (exact $B))) (local $A (ref $A)) (local $B (ref $B)) (drop @@ -38,26 +52,37 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct)) + ;; CHECK-NEXT: (type $A (descriptor $B (struct))) + ;; T_N_H: (rec + ;; T_N_H-NEXT: (type $A (struct)) (type $A (descriptor $B (struct))) - ;; CHECK: (type $B (struct)) + ;; CHECK: (type $B (describes $A (struct))) + ;; T_N_H: (type $B (struct)) (type $B (describes $A (struct))) ) - ;; CHECK: (type $2 (func)) + ;; CHECK: (type $2 (func)) + + ;; CHECK: (global $g anyref (struct.new_default $A + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: )) + ;; T_N_H: (type $2 (func)) + + ;; T_N_H: (global $g anyref (struct.new_default $A)) (global $g anyref (struct.new $A - (struct.new $B) + (ref.null $B) )) ;; CHECK: (func $test (type $2) ;; CHECK-NEXT: (local $A (ref $A)) ;; CHECK-NEXT: (local $B (ref $B)) - ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.new_default $A) - ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) - (func $test (param $nullable (ref null $B)) + ;; T_N_H: (func $test (type $2) + ;; T_N_H-NEXT: (local $A (ref $A)) + ;; T_N_H-NEXT: (local $B (ref $B)) + ;; T_N_H-NEXT: ) + (func $test (local $A (ref $A)) (local $B (ref $B)) ) From 7a51d7870b4b447b0b75508e8c5de700a3b7aec2 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 10:17:01 -0700 Subject: [PATCH 35/57] fix --- src/passes/GlobalTypeOptimization.cpp | 13 ++++++++++--- test/lit/passes/gto-desc.wast | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 25d30d6e3a5..7b06e1bcaec 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -160,10 +160,17 @@ struct GlobalTypeOptimization : public Pass { // We need info from struct.news in only one situation, so far: custom // descriptors, where setting a nullable descriptor is a dangerous write, - // one with effects, that we must track. (Otherwise, nothing in a struct.new - // can prevent removing fields or making them immutable.) + // one with effects, that we must track. if (module->features.hasCustomDescriptors()) { - functionNewInfos.combineInto(combinedSetGetInfos); + // Otherwise, nothing in a struct.new can prevent removing fields or + // making them immutable, so we must only copy over descriptor fields here + // (that is, other writes from a struct.new are skipped here; they do not + // cause fields to remain mutable, like struct.set writes). + for (auto& [func, infos] : functionNewInfos) { + for (auto& [type, info] : infos) { + combinedSetGetInfos[type].desc.combine(info.desc); + } + } } // Propagate information to super and subtypes on set/get infos: diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 65f023b91ec..749cbe4dd6d 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -498,7 +498,7 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct (field (mut i32)))) + ;; CHECK-NEXT: (type $A (struct (field i32))) (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) ;; CHECK: (type $B (struct)) (type $B (describes $A (struct (field (mut i32))))) @@ -586,7 +586,7 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct (field (mut i32)))) + ;; CHECK-NEXT: (type $A (struct (field i32))) (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) ;; CHECK: (type $B (struct)) (type $B (describes $A (struct (field (mut i32))))) From fed87f1f3867b26b91e6fd2e42061794eb3a6547 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 10:21:30 -0700 Subject: [PATCH 36/57] work --- src/passes/GlobalTypeOptimization.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 7b06e1bcaec..5ae40936cda 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -88,10 +88,10 @@ struct FieldInfoScanner HeapType type, Index index, FieldInfo& info) { - if (index == DescriptorIndex) { - if (!expr->type.isNullable()) { - return; - } + if (index == DescriptorIndex && expr->type.isNonNullable()) { + // A non-dangerous write to a descriptor, which as mentioned above, we do + // not track. + return; } info.noteWrite(); } @@ -158,14 +158,16 @@ struct GlobalTypeOptimization : public Pass { // Combine the data from the functions. functionSetGetInfos.combineInto(combinedSetGetInfos); + auto trapsNeverHappen = getPassOptions().trapsNeverHappen; + // We need info from struct.news in only one situation, so far: custom - // descriptors, where setting a nullable descriptor is a dangerous write, - // one with effects, that we must track. - if (module->features.hasCustomDescriptors()) { + // descriptors, where setting a nullable descriptor is a dangerous write (it + // might trap). We only need this if we care about traps. + if (module->features.hasCustomDescriptors() && !trapsNeverHappen) { // Otherwise, nothing in a struct.new can prevent removing fields or // making them immutable, so we must only copy over descriptor fields here // (that is, other writes from a struct.new are skipped here; they do not - // cause fields to remain mutable, like struct.set writes). + // cause fields to remain mutable, unlike struct.set writes). for (auto& [func, infos] : functionNewInfos) { for (auto& [type, info] : infos) { combinedSetGetInfos[type].desc.combine(info.desc); @@ -210,8 +212,6 @@ struct GlobalTypeOptimization : public Pass { std::unordered_set publicTypesSet(publicTypes.begin(), publicTypes.end()); - auto trapsNeverHappen = getPassOptions().trapsNeverHappen; - // 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 From a1d7068ee5dd049bf4089e7220f2ab6ea7c07fb8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 10:52:27 -0700 Subject: [PATCH 37/57] wip --- src/passes/GlobalTypeOptimization.cpp | 10 +++++++++- test/lit/passes/gto-desc-tnh.wast | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 5ae40936cda..72a08cb5e37 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -170,7 +170,15 @@ struct GlobalTypeOptimization : public Pass { // cause fields to remain mutable, unlike struct.set writes). for (auto& [func, infos] : functionNewInfos) { for (auto& [type, info] : infos) { - combinedSetGetInfos[type].desc.combine(info.desc); + if (info.desc.hasWrite) { + combinedSetGetInfos[type].noteWrite(); + + // We must also propagate these dangerous writes to describees. + // Imagine that $A, $A.desc are a pair of describee/descriptor, and + // $B, $B.desc as well with subtyping only between the descriptors, and *not* $A and $B. + // We still cannot remove the descriptor from either, if one has a + // dangerous write. + } } } } diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index e221d4e9047..e2baffe1f09 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -88,3 +88,27 @@ ) ) +;; $A and $B have descriptors, and the descriptors also subtype. +;; +;; $B is written a null descriptor, so we cannot optimize it when traps are +;; possible. This also prevents optimizations on $A: we cannot remove that +;; descriptor either, or its subtype would break. +;; +;; This tests subtyping of descriptors *without* subtyping of describees. +(module + (rec + (type $A (sub (descriptor $A.desc (struct)))) + (type $A.desc (sub (describes $A (struct )))) + + (type $B (sub (descriptor $B.desc (struct)))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + ) + + (func $test + (drop + (struct.new_default $B + (ref.null none) + ) + ) + ) +) From 0a4dc43c367b782c441a9d926670475e967586af Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:03:39 -0700 Subject: [PATCH 38/57] work --- src/ir/struct-utils.h | 58 ++++++++++++++++-- src/passes/GlobalTypeOptimization.cpp | 84 ++++++++++++++++++++------- 2 files changed, 118 insertions(+), 24 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index f208d624893..1632f37c2b0 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -310,8 +310,8 @@ struct StructScanner FunctionStructValuesMap& functionSetGetInfos; }; -// Helper class to propagate information about fields to sub- and/or super- -// classes in the type hierarchy. While propagating it calls a method +// Helper class to propagate information to sub- and/or super- classes in the +// type hierarchy. While propagating it calls a method // // to.combine(from) // @@ -328,18 +328,34 @@ template class TypeHierarchyPropagator { SubTypes subTypes; + // Propagate given a StructValuesMap, which means we need to take into + // account fields. void propagateToSuperTypes(StructValuesMap& infos) { propagate(infos, false, true); } - void propagateToSubTypes(StructValuesMap& infos) { propagate(infos, true, false); } - void propagateToSuperAndSubTypes(StructValuesMap& infos) { propagate(infos, true, true); } + // Propagate on a simpler map of structs and infos (that is, not using + // separate values for the fields, as StructValuesMap does). This is useful + // when not tracking individual fields, but something more general about + // types. + using StructMap = std::unordered_map; + + void propagateToSuperTypes(StructMap& infos) { + propagate(infos, false, true); + } + void propagateToSubTypes(StructMap& infos) { + propagate(infos, true, false); + } + void propagateToSuperAndSubTypes(StructMap& infos) { + propagate(infos, true, true); + } + private: void propagate(StructValuesMap& combinedInfos, bool toSubTypes, @@ -387,6 +403,40 @@ template class TypeHierarchyPropagator { } } } + + void propagate(StructMap& combinedInfos, + bool toSubTypes, + bool toSuperTypes) { + UniqueDeferredQueue work; + for (auto& [type, _] : combinedInfos) { + work.push(type); + } + while (!work.empty()) { + auto type = work.pop(); + auto& info = combinedInfos[type]; + + if (toSuperTypes) { + // Propagate to the supertype. + if (auto superType = type.getDeclaredSuperType()) { + auto& superInfo = combinedInfos[*superType]; + if (superInfo.combine(info)) { + work.push(*superType); + } + } + } + + if (toSubTypes) { + // Propagate shared fields to the subtypes. + auto numFields = type.getStruct().fields.size(); + for (auto subType : subTypes.getImmediateSubTypes(type)) { + auto& subInfo = combinedInfos[subType]; + if (subInfo.combine(info)) { + work.push(subType); + } + } + } + } + } }; } // namespace StructUtils diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 72a08cb5e37..bfaf971e7bc 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -28,6 +28,7 @@ #include "ir/names.h" #include "ir/ordering.h" #include "ir/struct-utils.h" +#include "ir/subtypes.h" #include "ir/type-updating.h" #include "ir/utils.h" #include "pass.h" @@ -148,6 +149,8 @@ struct GlobalTypeOptimization : public Pass { Fatal() << "GTO requires --closed-world"; } + auto trapsNeverHappen = getPassOptions().trapsNeverHappen; + // Find and analyze struct operations inside each function. StructUtils::FunctionStructValuesMap functionNewInfos(*module), functionSetGetInfos(*module); @@ -158,26 +161,18 @@ struct GlobalTypeOptimization : public Pass { // Combine the data from the functions. functionSetGetInfos.combineInto(combinedSetGetInfos); - auto trapsNeverHappen = getPassOptions().trapsNeverHappen; - - // We need info from struct.news in only one situation, so far: custom - // descriptors, where setting a nullable descriptor is a dangerous write (it - // might trap). We only need this if we care about traps. + // Custom descriptor handling: We need to look at struct.news, which + // normally we ignore (nothing in a struct.new can cause fields to remain + // mutable, or force the field to stay around. We cannot ignore them with CD + // because struct.news can now trap, and removing the descriptor could + // change things, so we must be careful. (Without traps, though, this is + // unnecessary.) if (module->features.hasCustomDescriptors() && !trapsNeverHappen) { - // Otherwise, nothing in a struct.new can prevent removing fields or - // making them immutable, so we must only copy over descriptor fields here - // (that is, other writes from a struct.new are skipped here; they do not - // cause fields to remain mutable, unlike struct.set writes). for (auto& [func, infos] : functionNewInfos) { for (auto& [type, info] : infos) { if (info.desc.hasWrite) { - combinedSetGetInfos[type].noteWrite(); - - // We must also propagate these dangerous writes to describees. - // Imagine that $A, $A.desc are a pair of describee/descriptor, and - // $B, $B.desc as well with subtyping only between the descriptors, and *not* $A and $B. - // We still cannot remove the descriptor from either, if one has a - // dangerous write. + // Copy the descriptor write to the info we will propagate below. + combinedSetGetInfos[type].desc.noteWrite(); } } } @@ -209,7 +204,8 @@ struct GlobalTypeOptimization : public Pass { // subtypes (as wasm only allows the type to differ if the fields are // immutable). Note that by making more things immutable we therefore // make it possible to apply more specific subtypes in subtype fields. - StructUtils::TypeHierarchyPropagator propagator(*module); + SubTypes subTypes(*module); + StructUtils::TypeHierarchyPropagator propagator(subTypes); auto dataFromSubsAndSupersMap = combinedSetGetInfos; propagator.propagateToSuperAndSubTypes(dataFromSubsAndSupersMap); auto dataFromSupersMap = std::move(combinedSetGetInfos); @@ -422,9 +418,57 @@ struct GlobalTypeOptimization : public Pass { } } - // Descriptor/describings are in pairs, so the size of these sets is equal, - // and we only need to check one below. - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + // Handle descriptor subtyping: + // + // A -> A.desc + // ^ + // B -> B.desc + // + // Here the descriptors subtype, but *not* the describees. We cannot + // remove A's descriptor without also removing $B's, so we need to propagate + // that "must remain a descriptor" property among descriptors. + if (!haveUnneededDescriptors.empty()) { + // Descriptor/describings are in pairs. + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + + struct DescriptorInfo { + // Whether this descriptor is needed - it must keep describing. + bool needed = false; + + bool combine(const DescriptorInfo& other) { + if (!needed && other.needed) { + needed = true; + return true; + } + return false; + } + }; + + TypeHierarchyPropagator descPropagator(subTypes); + + // Populate the initial data: Anything we did not see was unneeded, is + // needed. + TypeHierarchyPropagator::StructMap map; + for (auto type : subTypes.types) { + if (type.isStruct() && !haveUnneededDescribings.count(type)) { + // This descriptor type is needed. + map[type].needed = true; + } + } + + // Propagate. + descPropagator.propagateToSuperAndSubTypes(map); + + // Remove optimization opportunities that the propagation ruled out. + for (auto& [type, info] : map) { + if (info.needed) { + ehh + } + } + + // Descriptor/describings should still be in pairs. + assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); + } // If we found things that can be removed, remove them from instructions. // (Note that we must do this first, while we still have the old heap types From 90f1fb5a839a113ccb5aabe26bd21eccf82badb0 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:48:30 -0700 Subject: [PATCH 39/57] work --- src/ir/struct-utils.h | 1 - src/passes/GlobalTypeOptimization.cpp | 16 ++++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 1632f37c2b0..3d282697003 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -427,7 +427,6 @@ template class TypeHierarchyPropagator { if (toSubTypes) { // Propagate shared fields to the subtypes. - auto numFields = type.getStruct().fields.size(); for (auto subType : subTypes.getImmediateSubTypes(type)) { auto& subInfo = combinedInfos[subType]; if (subInfo.combine(info)) { diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index bfaf971e7bc..294bd0c89ba 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -444,13 +444,13 @@ struct GlobalTypeOptimization : public Pass { } }; - TypeHierarchyPropagator descPropagator(subTypes); + StructUtils::TypeHierarchyPropagator descPropagator(subTypes); - // Populate the initial data: Anything we did not see was unneeded, is - // needed. - TypeHierarchyPropagator::StructMap map; + // Populate the initial data: Any descriptor we did not see was unneeded, + // is needed. + StructUtils::TypeHierarchyPropagator::StructMap map; for (auto type : subTypes.types) { - if (type.isStruct() && !haveUnneededDescribings.count(type)) { + if (type.getDescribedType() && !haveUnneededDescribings.count(type)) { // This descriptor type is needed. map[type].needed = true; } @@ -462,7 +462,11 @@ struct GlobalTypeOptimization : public Pass { // Remove optimization opportunities that the propagation ruled out. for (auto& [type, info] : map) { if (info.needed) { - ehh + if (haveUnneededDescribings.erase(type)) { + auto described = type.getDescribedType(); + assert(described); + haveUnneededDescriptors.erase(*described); + } } } From 6b722ef1ddd5951a0e64bc9ab2f0ee2997a01d64 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:50:33 -0700 Subject: [PATCH 40/57] work --- test/lit/passes/gto-desc-tnh.wast | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index e2baffe1f09..22b4442c52b 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -97,13 +97,39 @@ ;; This tests subtyping of descriptors *without* subtyping of describees. (module (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + ;; T_N_H: (rec + ;; T_N_H-NEXT: (type $A (sub (struct))) (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + ;; T_N_H: (type $A.desc (sub (struct))) (type $A.desc (sub (describes $A (struct )))) + ;; CHECK: (type $B (sub (descriptor $B.desc (struct)))) + ;; T_N_H: (type $B (sub (struct))) (type $B (sub (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct)))) + ;; T_N_H: (type $B.desc (sub $A.desc (struct))) (type $B.desc (sub $A.desc (describes $B (struct)))) ) + ;; CHECK: (type $4 (func)) + + ;; CHECK: (func $test (type $4) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (struct.new_default $B + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; T_N_H: (type $4 (func)) + + ;; T_N_H: (func $test (type $4) + ;; T_N_H-NEXT: (drop + ;; T_N_H-NEXT: (struct.new_default $B) + ;; T_N_H-NEXT: ) + ;; T_N_H-NEXT: ) (func $test (drop (struct.new_default $B From 9e1409af475f1297bdb299419161ab116c22e245 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:51:13 -0700 Subject: [PATCH 41/57] work --- test/lit/passes/gto-desc-tnh.wast | 62 +++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index 22b4442c52b..2ae02cdc8e4 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -138,3 +138,65 @@ ) ) ) + +;; A similar situation, but now the thing that stops optimizing $B is not a +;; null descriptor but a use. We cannot optimize even without traps. +;; Subtyping of descriptors *without* subtyping of describees. +;; +;; $B's descriptor seems removable, but the subtyping of the descriptors +;; prevents this. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + ;; T_N_H: (rec + ;; T_N_H-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + ;; T_N_H: (type $A.desc (sub (describes $A (struct)))) + (type $A.desc (sub (describes $A (struct )))) + + ;; CHECK: (type $B (sub (descriptor $B.desc (struct)))) + ;; T_N_H: (type $B (sub (descriptor $B.desc (struct)))) + (type $B (sub (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub $A.desc (describes $B (struct)))) + ;; T_N_H: (type $B.desc (sub $A.desc (describes $B (struct)))) + (type $B.desc (sub $A.desc (describes $B (struct)))) + ) + + ;; CHECK: (type $4 (func)) + + ;; CHECK: (func $test (type $4) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $A + ;; CHECK-NEXT: (struct.new_default $A + ;; CHECK-NEXT: (struct.new_default $A.desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; T_N_H: (type $4 (func)) + + ;; T_N_H: (func $test (type $4) + ;; T_N_H-NEXT: (local $B (ref $B)) + ;; T_N_H-NEXT: (drop + ;; T_N_H-NEXT: (ref.get_desc $A + ;; T_N_H-NEXT: (struct.new_default $A + ;; T_N_H-NEXT: (struct.new_default $A.desc) + ;; T_N_H-NEXT: ) + ;; T_N_H-NEXT: ) + ;; T_N_H-NEXT: ) + ;; T_N_H-NEXT: ) + (func $test + (local $B (ref $B)) ;; keep $B alive + (drop + (ref.get_desc $A + (struct.new $A + (struct.new $A.desc) + ) + ) + ) + ) +) + From c7744419ff936e575169ea9b76973dbfe81b6f6b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:53:50 -0700 Subject: [PATCH 42/57] work --- src/ir/struct-utils.h | 8 ++------ src/passes/GlobalTypeOptimization.cpp | 3 ++- test/lit/passes/gto-desc-tnh.wast | 6 +++++- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 3d282697003..f5802d9105d 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -349,9 +349,7 @@ template class TypeHierarchyPropagator { void propagateToSuperTypes(StructMap& infos) { propagate(infos, false, true); } - void propagateToSubTypes(StructMap& infos) { - propagate(infos, true, false); - } + void propagateToSubTypes(StructMap& infos) { propagate(infos, true, false); } void propagateToSuperAndSubTypes(StructMap& infos) { propagate(infos, true, true); } @@ -404,9 +402,7 @@ template class TypeHierarchyPropagator { } } - void propagate(StructMap& combinedInfos, - bool toSubTypes, - bool toSuperTypes) { + void propagate(StructMap& combinedInfos, bool toSubTypes, bool toSuperTypes) { UniqueDeferredQueue work; for (auto& [type, _] : combinedInfos) { work.push(type); diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 294bd0c89ba..633bc62e979 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -444,7 +444,8 @@ struct GlobalTypeOptimization : public Pass { } }; - StructUtils::TypeHierarchyPropagator descPropagator(subTypes); + StructUtils::TypeHierarchyPropagator descPropagator( + subTypes); // Populate the initial data: Any descriptor we did not see was unneeded, // is needed. diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index 2ae02cdc8e4..19479f2aab9 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -88,7 +88,11 @@ ) ) -;; $A and $B have descriptors, and the descriptors also subtype. +;; $A and $B have descriptors, and the descriptors also subtype: +;; +;; A -> A.desc +;; ^ +;; B -> B.desc ;; ;; $B is written a null descriptor, so we cannot optimize it when traps are ;; possible. This also prevents optimizations on $A: we cannot remove that From 08cc60bf101d4ecd5adbaa0a3712e7f9bf675aba Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 12:59:17 -0700 Subject: [PATCH 43/57] feedback: comments --- src/passes/ConstantFieldPropagation.cpp | 1 + src/passes/TypeRefining.cpp | 1 + 2 files changed, 2 insertions(+) diff --git a/src/passes/ConstantFieldPropagation.cpp b/src/passes/ConstantFieldPropagation.cpp index b76d631ed83..75f74559f3e 100644 --- a/src/passes/ConstantFieldPropagation.cpp +++ b/src/passes/ConstantFieldPropagation.cpp @@ -444,6 +444,7 @@ struct PCVScanner void noteCopy(HeapType type, Index index, PossibleConstantValues& info) { if (index == DescriptorIndex) { + // We cannot continue on below, where we index into the vector of values. return; } diff --git a/src/passes/TypeRefining.cpp b/src/passes/TypeRefining.cpp index 49ced6d2038..3ec7d2923a8 100644 --- a/src/passes/TypeRefining.cpp +++ b/src/passes/TypeRefining.cpp @@ -62,6 +62,7 @@ struct FieldInfoScanner Index index, FieldInfo& info) { if (index == DescriptorIndex) { + // We cannot continue on below, where we index into the vector of values. return; } From a490a8ef65605d7ee7b5acccc628c8977d0ac7b1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 13:05:17 -0700 Subject: [PATCH 44/57] feedback: remove parallel sets --- src/passes/GlobalTypeOptimization.cpp | 32 +++++++++++---------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 633bc62e979..f73396e3d5f 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -135,10 +135,8 @@ struct GlobalTypeOptimization : public Pass { static const Index RemovedField = Index(-1); std::unordered_map> indexesAfterRemovals; - // The types that no longer need a descriptor, and that no longer need to - // describe. + // The types that no longer need a descriptor. std::unordered_set haveUnneededDescriptors; - std::unordered_set haveUnneededDescribings; void run(Module* module) override { if (!module->features.hasGC()) { @@ -413,7 +411,6 @@ struct GlobalTypeOptimization : public Pass { if (!dataFromSubsAndSupers.desc.hasRead && (!dataFromSubsAndSupers.desc.hasWrite || trapsNeverHappen)) { haveUnneededDescriptors.insert(type); - haveUnneededDescribings.insert(*desc); } } } @@ -428,8 +425,6 @@ struct GlobalTypeOptimization : public Pass { // remove A's descriptor without also removing $B's, so we need to propagate // that "must remain a descriptor" property among descriptors. if (!haveUnneededDescriptors.empty()) { - // Descriptor/describings are in pairs. - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); struct DescriptorInfo { // Whether this descriptor is needed - it must keep describing. @@ -451,9 +446,11 @@ struct GlobalTypeOptimization : public Pass { // is needed. StructUtils::TypeHierarchyPropagator::StructMap map; for (auto type : subTypes.types) { - if (type.getDescribedType() && !haveUnneededDescribings.count(type)) { - // This descriptor type is needed. - map[type].needed = true; + if (auto desc = type.getDescriptorType()) { + if (!haveUnneededDescriptors.count(type)) { + // This descriptor type is needed. + map[*desc].needed = true; + } } } @@ -463,16 +460,11 @@ struct GlobalTypeOptimization : public Pass { // Remove optimization opportunities that the propagation ruled out. for (auto& [type, info] : map) { if (info.needed) { - if (haveUnneededDescribings.erase(type)) { - auto described = type.getDescribedType(); - assert(described); - haveUnneededDescriptors.erase(*described); - } + auto described = type.getDescribedType(); + assert(described); + haveUnneededDescriptors.erase(*described); } } - - // Descriptor/describings should still be in pairs. - assert(haveUnneededDescriptors.size() == haveUnneededDescribings.size()); } // If we found things that can be removed, remove them from instructions. @@ -564,8 +556,10 @@ struct GlobalTypeOptimization : public Pass { } // Remove an unneeded describes. - if (parent.haveUnneededDescribings.count(oldType)) { - typeBuilder.setDescribed(i, std::nullopt); + if (auto described = oldType.getDescribedType()) { + if (parent.haveUnneededDescriptors.count(*described)) { + typeBuilder.setDescribed(i, std::nullopt); + } } } }; From 88340c309f8a31b49e6ec07ead49ef9ca4c2e9d9 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 13:15:17 -0700 Subject: [PATCH 45/57] feedback: test field names --- test/lit/passes/gto-desc.wast | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 749cbe4dd6d..e022fe0f347 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -498,8 +498,8 @@ (module (rec ;; CHECK: (rec - ;; CHECK-NEXT: (type $A (struct (field i32))) - (type $A (descriptor $B (struct (field (mut i32)) (field (mut i32)) (field (mut i32))))) + ;; CHECK-NEXT: (type $A (struct (field $c i32))) + (type $A (descriptor $B (struct (field $a (mut i32)) (field $b (mut i32)) (field $c (mut i32))))) ;; CHECK: (type $B (struct)) (type $B (describes $A (struct (field (mut i32))))) ) @@ -533,7 +533,7 @@ ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) ;; CHECK-NEXT: (drop - ;; CHECK-NEXT: (struct.get $A 0 + ;; CHECK-NEXT: (struct.get $A $c ;; CHECK-NEXT: (local.get $A) ;; CHECK-NEXT: ) ;; CHECK-NEXT: ) @@ -564,13 +564,13 @@ ) ) ;; Write only the middle field (we can remove it as write-only). - (struct.set $A 1 + (struct.set $A $b (local.get $A) (i32.const 42) ) ;; Read only the last field. We can make it immutable. (drop - (struct.get $A 2 + (struct.get $A $c (local.get $A) ) ) From 88cc7dcd47693eb7c39052e1662dcda8e70c8d4d Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 13:21:34 -0700 Subject: [PATCH 46/57] feedback: comment --- src/ir/struct-utils.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index f5802d9105d..fa47d41d457 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -45,7 +45,14 @@ template struct StructValues : public std::vector { return std::vector::operator[](index); } - // Store the descriptor as another field. + // Store the descriptor as another field. (This could be a std::optional to + // indicate that the descriptor's existence depends on the type, but that + // would add overhead & code clutter (type checks). If there is no descriptor, + // this will just hang around with the default values, not harming anything + // except perhaps for looking a little odd during debugging. And whenever we + // combine() a non-existent descriptor, we are doing unneeded work, but the + // data here is typically just a few bools, so it is simpler and likely + // faster to just copy those rather than check if the type has a descriptor.) T desc; }; From 57f120cd6a3e95868ff022a6d34b0991b53720e1 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 15:39:57 -0700 Subject: [PATCH 47/57] tlively's idea? --- src/passes/GlobalTypeOptimization.cpp | 43 +++++++++++++++++++-------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index f73396e3d5f..32a4c405308 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -136,7 +136,7 @@ struct GlobalTypeOptimization : public Pass { std::unordered_map> indexesAfterRemovals; // The types that no longer need a descriptor. - std::unordered_set haveUnneededDescriptors; + std::unordered_map> haveUnneededDescriptors; void run(Module* module) override { if (!module->features.hasGC()) { @@ -408,9 +408,9 @@ struct GlobalTypeOptimization : public Pass { // could trap, we'd have no easy way to remove it in a global scope. // TODO: We could check and handle the global scope specifically, but // the trapsNeverHappen flag avoids this problem entirely anyhow. - if (!dataFromSubsAndSupers.desc.hasRead && - (!dataFromSubsAndSupers.desc.hasWrite || trapsNeverHappen)) { - haveUnneededDescriptors.insert(type); + if (!dataFromSupers.desc.hasRead && + (!dataFromSupers.desc.hasWrite || trapsNeverHappen)) { + haveUnneededDescriptors[type] = std::nullopt; } } } @@ -422,12 +422,12 @@ struct GlobalTypeOptimization : public Pass { // B -> B.desc // // Here the descriptors subtype, but *not* the describees. We cannot - // remove A's descriptor without also removing $B's, so we need to propagate - // that "must remain a descriptor" property among descriptors. + // remove A's descriptor by itself, not without also removing B's, so we + // need to do something more (see below). if (!haveUnneededDescriptors.empty()) { - + // To find problem situations, we'll progagate the property of a + // descriptor being needed because of descriptor subtyping. struct DescriptorInfo { - // Whether this descriptor is needed - it must keep describing. bool needed = false; bool combine(const DescriptorInfo& other) { @@ -457,12 +457,24 @@ struct GlobalTypeOptimization : public Pass { // Propagate. descPropagator.propagateToSuperAndSubTypes(map); - // Remove optimization opportunities that the propagation ruled out. + // Find optimization opportunities that the propagation ruled out. for (auto& [type, info] : map) { if (info.needed) { auto described = type.getDescribedType(); assert(described); - haveUnneededDescriptors.erase(*described); + if (haveUnneededDescriptors.count(*described)) { + // We are in a situation like on the left: + // + // A -> A.desc A A.desc <- A2 + // ^ => ^ + // B -> B.desc B -> B.desc + // + // We want to remove A's descriptor, but cannot remove B's. To do + // that, we add a new type A2 for A.desc to describe, which keeps + // the property that A.desc and B.desc are a parent/child pair of + // descriptors, which is necessary for validation. + haveUnneededDescriptors[*described] = HeapType(described->getStruct()); + } } } } @@ -555,10 +567,15 @@ struct GlobalTypeOptimization : public Pass { typeBuilder.setDescriptor(i, std::nullopt); } - // Remove an unneeded describes. + // Remove or replace describings. if (auto described = oldType.getDescribedType()) { - if (parent.haveUnneededDescriptors.count(*described)) { - typeBuilder.setDescribed(i, std::nullopt); + auto iter = parent.haveUnneededDescriptors.find(*described); + if (iter != parent.haveUnneededDescriptors.end()) { + auto value = iter->second; + if (value) { + value = getTempHeapType(*value); + } + typeBuilder.setDescribed(i, value); } } } From 8a4286a217007e327a4b281ea7049acda949949f Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 15:40:00 -0700 Subject: [PATCH 48/57] Revert "tlively's idea?" This reverts commit 57f120cd6a3e95868ff022a6d34b0991b53720e1. --- src/passes/GlobalTypeOptimization.cpp | 43 ++++++++------------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 32a4c405308..f73396e3d5f 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -136,7 +136,7 @@ struct GlobalTypeOptimization : public Pass { std::unordered_map> indexesAfterRemovals; // The types that no longer need a descriptor. - std::unordered_map> haveUnneededDescriptors; + std::unordered_set haveUnneededDescriptors; void run(Module* module) override { if (!module->features.hasGC()) { @@ -408,9 +408,9 @@ struct GlobalTypeOptimization : public Pass { // could trap, we'd have no easy way to remove it in a global scope. // TODO: We could check and handle the global scope specifically, but // the trapsNeverHappen flag avoids this problem entirely anyhow. - if (!dataFromSupers.desc.hasRead && - (!dataFromSupers.desc.hasWrite || trapsNeverHappen)) { - haveUnneededDescriptors[type] = std::nullopt; + if (!dataFromSubsAndSupers.desc.hasRead && + (!dataFromSubsAndSupers.desc.hasWrite || trapsNeverHappen)) { + haveUnneededDescriptors.insert(type); } } } @@ -422,12 +422,12 @@ struct GlobalTypeOptimization : public Pass { // B -> B.desc // // Here the descriptors subtype, but *not* the describees. We cannot - // remove A's descriptor by itself, not without also removing B's, so we - // need to do something more (see below). + // remove A's descriptor without also removing $B's, so we need to propagate + // that "must remain a descriptor" property among descriptors. if (!haveUnneededDescriptors.empty()) { - // To find problem situations, we'll progagate the property of a - // descriptor being needed because of descriptor subtyping. + struct DescriptorInfo { + // Whether this descriptor is needed - it must keep describing. bool needed = false; bool combine(const DescriptorInfo& other) { @@ -457,24 +457,12 @@ struct GlobalTypeOptimization : public Pass { // Propagate. descPropagator.propagateToSuperAndSubTypes(map); - // Find optimization opportunities that the propagation ruled out. + // Remove optimization opportunities that the propagation ruled out. for (auto& [type, info] : map) { if (info.needed) { auto described = type.getDescribedType(); assert(described); - if (haveUnneededDescriptors.count(*described)) { - // We are in a situation like on the left: - // - // A -> A.desc A A.desc <- A2 - // ^ => ^ - // B -> B.desc B -> B.desc - // - // We want to remove A's descriptor, but cannot remove B's. To do - // that, we add a new type A2 for A.desc to describe, which keeps - // the property that A.desc and B.desc are a parent/child pair of - // descriptors, which is necessary for validation. - haveUnneededDescriptors[*described] = HeapType(described->getStruct()); - } + haveUnneededDescriptors.erase(*described); } } } @@ -567,15 +555,10 @@ struct GlobalTypeOptimization : public Pass { typeBuilder.setDescriptor(i, std::nullopt); } - // Remove or replace describings. + // Remove an unneeded describes. if (auto described = oldType.getDescribedType()) { - auto iter = parent.haveUnneededDescriptors.find(*described); - if (iter != parent.haveUnneededDescriptors.end()) { - auto value = iter->second; - if (value) { - value = getTempHeapType(*value); - } - typeBuilder.setDescribed(i, value); + if (parent.haveUnneededDescriptors.count(*described)) { + typeBuilder.setDescribed(i, std::nullopt); } } } From 567ecbd952a2b86227051709e82f7af7c09ddb46 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 15:46:29 -0700 Subject: [PATCH 49/57] Optimize siblings --- src/passes/GlobalTypeOptimization.cpp | 14 +++++----- test/lit/passes/gto-desc.wast | 38 +++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index f73396e3d5f..6ecf809dfc9 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -402,14 +402,16 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { - // To remove a descriptor, it must not be used in either subtypes or - // supertypes, to not break validation. It must also have no write (see - // above, we note only dangerous writes which might trap), as if it - // could trap, we'd have no easy way to remove it in a global scope. + // To remove a descriptor, it must not be used in subtypes. It must also + // have no write (see above, we note only dangerous writes which might + // trap), as if it could trap, we'd have no easy way to remove it in a + // global scope. // TODO: We could check and handle the global scope specifically, but // the trapsNeverHappen flag avoids this problem entirely anyhow. - if (!dataFromSubsAndSupers.desc.hasRead && - (!dataFromSubsAndSupers.desc.hasWrite || trapsNeverHappen)) { + // + // This does not handle descriptor subtyping, see below. + if (!dataFromSupers.desc.hasRead && + (!dataFromSupers.desc.hasWrite || trapsNeverHappen)) { haveUnneededDescriptors.insert(type); } } diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index e022fe0f347..6f0389db9be 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1026,3 +1026,41 @@ ) ) +;; Sibling types $A and $B. The supertype that connects them should not stop us +;; from optimizing $B here, even though $A cannot be. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $super (sub (struct))) + (type $super (sub (struct))) + + ;; CHECK: (type $A (sub $super (descriptor $A.desc (struct)))) + (type $A (sub $super (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (describes $A (struct))) + (type $A.desc (describes $A (struct))) + + ;; CHECK: (type $B (sub $super (struct))) + (type $B (sub $super (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (struct)) + (type $B.desc (describes $B (struct))) + ) + + ;; CHECK: (type $5 (func (param (ref $A)))) + + ;; CHECK: (func $test (type $5) (param $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (drop + ;; CHECK-NEXT: (ref.get_desc $A + ;; CHECK-NEXT: (local.get $A) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (param $A (ref $A)) + (local $B (ref $B)) + (drop + (ref.get_desc $A + (local.get $A) + ) + ) + ) +) From 2950719608329a9ff96ce73adeaf2333bf64b2f8 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Fri, 29 Aug 2025 16:05:17 -0700 Subject: [PATCH 50/57] fix nested global effects --- src/passes/GlobalTypeOptimization.cpp | 9 +++-- test/lit/passes/gto-desc-tnh.wast | 52 +++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 6ecf809dfc9..b7ac4db763c 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -653,8 +653,13 @@ struct GlobalTypeOptimization : public Pass { } if (removeDesc) { - // We already handled the case of a possible trap here, so we can just - // remove the descriptor. + // We already handled the case of a possible trap here, so we can + // remove the descriptor, but must be careful of nested effects (our + // descriptor may be ok to remove, but a nested struct.new may not). + if (!func && + EffectAnalyzer(getPassOptions(), *getModule(), curr->desc).trap) { + removedTrappingInits.push_back(curr->desc); + } curr->desc = nullptr; } } diff --git a/test/lit/passes/gto-desc-tnh.wast b/test/lit/passes/gto-desc-tnh.wast index 19479f2aab9..dbf4f6ed938 100644 --- a/test/lit/passes/gto-desc-tnh.wast +++ b/test/lit/passes/gto-desc-tnh.wast @@ -204,3 +204,55 @@ ) ) +;; A chain of descriptors, where we can remove the outer one, but must be +;; careful not to remove nested children. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (struct)) + ;; T_N_H: (rec + ;; T_N_H-NEXT: (type $A (struct)) + (type $A (descriptor $B (struct))) + ;; CHECK: (type $B (descriptor $C (struct))) + ;; T_N_H: (type $B (struct)) + (type $B (describes $A (descriptor $C (struct)))) + ;; CHECK: (type $C (describes $B (struct))) + ;; T_N_H: (type $C (struct)) + (type $C (describes $B (struct))) + ) + + ;; CHECK: (type $3 (func)) + + ;; CHECK: (global $g anyref (struct.new_default $A)) + ;; T_N_H: (type $3 (func)) + + ;; T_N_H: (global $g anyref (struct.new_default $A)) + (global $g anyref + (struct.new $A ;; The outer struct.new $A is ok to remove, + (struct.new $B ;; but the inner struct.new $B is not, due + (ref.null $C) ;; to its null descriptor. We keep the inner + ) ;; one around as a new global later (but not + ) ;; if traps cannot happen). + ) + + ;; CHECK: (global $gto-removed-0 (ref (exact $B)) (struct.new_default $B + ;; CHECK-NEXT: (ref.null none) + ;; CHECK-NEXT: )) + + ;; CHECK: (func $test (type $3) + ;; CHECK-NEXT: (local $A (ref $A)) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (local $C (ref $C)) + ;; CHECK-NEXT: ) + ;; T_N_H: (func $test (type $3) + ;; T_N_H-NEXT: (local $A (ref $A)) + ;; T_N_H-NEXT: (local $B (ref $B)) + ;; T_N_H-NEXT: (local $C (ref $C)) + ;; T_N_H-NEXT: ) + (func $test + (local $A (ref $A)) + (local $B (ref $B)) + (local $C (ref $C)) + ) +) + From 03ff3e5dfda16b33561748c453dc23f053ae9a3e Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 09:48:11 -0700 Subject: [PATCH 51/57] fix --- src/ir/struct-utils.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index fa47d41d457..0ffb4898772 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -275,11 +275,13 @@ struct StructScanner void handleDescRead(Expression* ref) { auto type = ref->type; - if (type == Type::unreachable || type.isNull()) { + if (type == Type::unreachable) { return; } - auto heapType = type.getHeapType(); + if (!heapType.isStruct()) { + return; + } self().noteRead(heapType, DescriptorIndex, functionSetGetInfos[this->getFunction()][heapType].desc); From 55e70656391bc2626cc10cade344a6931a55352b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 10:06:05 -0700 Subject: [PATCH 52/57] fix casts+test --- src/ir/struct-utils.h | 27 +++++++++++++++---------- test/lit/passes/gto-desc.wast | 37 +++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 10 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 0ffb4898772..8e97a5ff459 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -260,31 +260,38 @@ struct StructScanner void visitRefCast(RefCast* curr) { if (curr->desc) { - handleDescRead(curr->ref); + // We may try to read a descriptor from anything arriving in |curr->ref|, + // but the only things that matter are the things we cast to: other types + // can lack a descriptor, and are skipped anyhow. So the only effective + // read is of the cast type. + handleDescRead(curr->getCastType()); } } - void visitRefGetDesc(RefGetDesc* curr) { handleDescRead(curr->ref); } - void visitBrOn(BrOn* curr) { if (curr->desc && (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail)) { - handleDescRead(curr->ref); + handleDescRead(curr->getCastType()); } } - void handleDescRead(Expression* ref) { - auto type = ref->type; + void visitRefGetDesc(RefGetDesc* curr) { + // Unlike a cast, any thing in |curr->ref| may be read from. + handleDescRead(curr->ref->type); + } + + void handleDescRead(Type type) { if (type == Type::unreachable) { return; } auto heapType = type.getHeapType(); - if (!heapType.isStruct()) { + if (heapType.isStruct()) { + // Any subtype of the reference here may be read from. + self().noteRead(heapType, + DescriptorIndex, + functionSetGetInfos[this->getFunction()][heapType].desc); return; } - self().noteRead(heapType, - DescriptorIndex, - functionSetGetInfos[this->getFunction()][heapType].desc); } void diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 6f0389db9be..515177e84ab 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1064,3 +1064,40 @@ ) ) ) + +;; ref.cast_desc on a basic supertype. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $A (sub (descriptor $A.desc (struct)))) + (type $A (sub (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (sub (describes $A (struct)))) + (type $A.desc (sub (describes $A (struct)))) + + ;; CHECK: (type $B (sub (struct))) + (type $B (sub (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (sub (struct))) + (type $B.desc (sub (describes $B (struct)))) + ) + + ;; CHECK: (type $4 (func (param anyref (ref (exact $A.desc))) (result (ref (exact $A))))) + + ;; CHECK: (func $cast_anyref (type $4) (param $ref anyref) (param $desc (ref (exact $A.desc))) (result (ref (exact $A))) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (ref.cast_desc (ref (exact $A)) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (local.get $desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $cast_anyref (param $ref anyref) (param $desc (ref (exact $A.desc))) (result (ref (exact $A))) + (local $B (ref $B)) + ;; The cast input is anyref. That means any struct could be sent in, but we + ;; only need to prevent optimization of $A, who is the cast output. $B can + ;; still be optimized. + (ref.cast_desc (ref (exact $A)) + (local.get $ref) + (local.get $desc) + ) + ) +) + From d40699028d3328b87fba91d6f4576ad81ed730bb Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 10:34:56 -0700 Subject: [PATCH 53/57] test --- test/lit/passes/gto-desc.wast | 40 ++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 515177e84ab..c6adc243470 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1065,7 +1065,45 @@ ) ) -;; ref.cast_desc on a basic supertype. +;; As above, but now with a cast of the supertype. We can still optimize $B but +;; not $A. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $super (sub (struct))) + (type $super (sub (struct))) + + ;; CHECK: (type $A (sub $super (descriptor $A.desc (struct)))) + (type $A (sub $super (descriptor $A.desc (struct)))) + ;; CHECK: (type $A.desc (describes $A (struct))) + (type $A.desc (describes $A (struct))) + + ;; CHECK: (type $B (sub $super (struct))) + (type $B (sub $super (descriptor $B.desc (struct)))) + ;; CHECK: (type $B.desc (struct)) + (type $B.desc (describes $B (struct))) + ) + + ;; CHECK: (type $5 (func (param (ref $super) (ref (exact $A.desc))) (result anyref))) + + ;; CHECK: (func $test (type $5) (param $ref (ref $super)) (param $desc (ref (exact $A.desc))) (result anyref) + ;; CHECK-NEXT: (local $B (ref $B)) + ;; CHECK-NEXT: (ref.cast_desc (ref (exact $A)) + ;; CHECK-NEXT: (local.get $ref) + ;; CHECK-NEXT: (local.get $desc) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $test (param $ref (ref $super)) (param $desc (ref (exact $A.desc))) (result anyref) + (local $B (ref $B)) + (ref.cast_desc (ref (exact $A)) + (local.get $ref) + (local.get $desc) + ) + ) +) + +;; As above, but now with a cast of a basic type. We can still optimize $B but +;; not $A. (module (rec ;; CHECK: (rec From b240e30ba2b558e2c236cfe9c2edcd0f77e2eae4 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 10:37:23 -0700 Subject: [PATCH 54/57] Update test/lit/passes/gto-desc.wast Co-authored-by: Thomas Lively --- test/lit/passes/gto-desc.wast | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index c6adc243470..465bfaf392b 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1027,7 +1027,7 @@ ) ;; Sibling types $A and $B. The supertype that connects them should not stop us -;; from optimizing $B here, even though $A cannot be. +;; from optimizing $B here, even though $A cannot be optimized. (module (rec ;; CHECK: (rec From d0ec85a39e57408418857519eefd6aab811b8e9b Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 10:41:19 -0700 Subject: [PATCH 55/57] Do not propagate a descriptor to a super without one --- src/ir/struct-utils.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 8e97a5ff459..12156eff8d9 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -392,8 +392,9 @@ template class TypeHierarchyPropagator { work.push(*superType); } } - // Propagate the descriptor. - if (superInfos.desc.combine(infos.desc)) { + // Propagate the descriptor to the super, if the super has one. + if (superType->getDescriptorType() && + superInfos.desc.combine(infos.desc)) { work.push(*superType); } } From 46634b35c63abf9f15c7362f230ac0b795ae1574 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 10:46:41 -0700 Subject: [PATCH 56/57] TODO --- src/passes/GlobalTypeOptimization.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index b7ac4db763c..6b6b90b245c 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -460,6 +460,16 @@ struct GlobalTypeOptimization : public Pass { descPropagator.propagateToSuperAndSubTypes(map); // Remove optimization opportunities that the propagation ruled out. + // TODO: We could do better here, + // + // A -> A.desc A A.desc <- A2 + // ^ => ^ + // B -> B.desc B -> B.desc + // + // Starting from the left, we can remove A's descriptor *but keep A.desc + // as being a descriptor*, by making it describe a new type A2. That would + // keep subtyping working for the descriptors, and later passes could + // remove the unused A2. for (auto& [type, info] : map) { if (info.needed) { auto described = type.getDescribedType(); From eb266ee7f394003789032878678994ec2d898037 Mon Sep 17 00:00:00 2001 From: Alon Zakai Date: Tue, 2 Sep 2025 11:19:40 -0700 Subject: [PATCH 57/57] Update src/ir/struct-utils.h Co-authored-by: Thomas Lively --- src/ir/struct-utils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ir/struct-utils.h b/src/ir/struct-utils.h index 12156eff8d9..6db4e7f9e70 100644 --- a/src/ir/struct-utils.h +++ b/src/ir/struct-utils.h @@ -276,7 +276,7 @@ struct StructScanner } void visitRefGetDesc(RefGetDesc* curr) { - // Unlike a cast, any thing in |curr->ref| may be read from. + // Unlike a cast, anything in |curr->ref| may be read from. handleDescRead(curr->ref->type); }