From 36d6b5362ace50c68ee1248ce5d000fed97966c3 Mon Sep 17 00:00:00 2001 From: Thomas Lively Date: Thu, 11 Sep 2025 13:04:05 -0700 Subject: [PATCH] [Custom Descriptors] Fix GTO descriptor optimization We were being too aggressive about removing unused descriptors. In particular, we were removing descriptors from subtypes of public types that themselves had descriptors, which caused invalid subtyping when rebuilding the types. Fix the problem by only removing descriptors from a type when we know we can remove the descriptor (if any) from its supertype (if any) as well. --- src/passes/GlobalTypeOptimization.cpp | 18 ++++-- test/lit/passes/gto-desc.wast | 84 +++++++++++++++++++++++++++ 2 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/passes/GlobalTypeOptimization.cpp b/src/passes/GlobalTypeOptimization.cpp index 8923ce9ce8f..281127067be 100644 --- a/src/passes/GlobalTypeOptimization.cpp +++ b/src/passes/GlobalTypeOptimization.cpp @@ -407,16 +407,22 @@ struct GlobalTypeOptimization : public Pass { // Process the descriptor. if (auto desc = type.getDescriptorType()) { - // 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. + // To remove a descriptor, it must not be read via supertypes and it + // must not have to remain in supertypes because e.g. they are public. + // 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. // // This does not handle descriptor subtyping, see below. - if (!dataFromSupers.desc.hasRead && - (!dataFromSupers.desc.hasWrite || trapsNeverHappen)) { + auto super = type.getDeclaredSuperType(); + bool superNeedsDescriptor = super && super->getDescriptorType() && + !haveUnneededDescriptors.count(*super); + bool descriptorIsUsed = + dataFromSupers.desc.hasRead || + (dataFromSupers.desc.hasWrite && !trapsNeverHappen); + if (!superNeedsDescriptor && !descriptorIsUsed) { haveUnneededDescriptors.insert(type); } } diff --git a/test/lit/passes/gto-desc.wast b/test/lit/passes/gto-desc.wast index 9bf1bf7f1df..5e441efcde9 100644 --- a/test/lit/passes/gto-desc.wast +++ b/test/lit/passes/gto-desc.wast @@ -1027,6 +1027,90 @@ ) ) +;; Here we cannot optimize with placeholders because the public supertype needs +;; to keep its descriptor and its subtypes therefore need to keep their related +;; descriptors. +(module + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $public (sub (descriptor $public.desc (struct)))) + (type $public (sub (descriptor $public.desc (struct)))) + ;; CHECK: (type $public.desc (sub (describes $public (struct)))) + (type $public.desc (sub (describes $public (struct)))) + ) + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $unused (sub $public (descriptor $unused.desc (struct)))) + (type $unused (sub $public (descriptor $unused.desc (struct)))) + ;; CHECK: (type $unused.desc (sub $public.desc (describes $unused (struct)))) + (type $unused.desc (sub $public.desc (describes $unused (struct)))) + + ;; CHECK: (type $used (sub $unused (descriptor $used.desc (struct)))) + (type $used (sub $unused (descriptor $used.desc (struct)))) + ;; CHECK: (type $used.desc (sub $unused.desc (describes $used (struct)))) + (type $used.desc (sub $unused.desc (describes $used (struct)))) + ) + + ;; CHECK: (type $6 (func (param (ref $used)) (result (ref $used.desc)))) + + ;; CHECK: (global $public (ref null $public) (ref.null none)) + (global $public (export "public") (ref null $public) (ref.null none)) + + ;; CHECK: (export "public" (global $public)) + + ;; CHECK: (func $use (type $6) (param $used (ref $used)) (result (ref $used.desc)) + ;; CHECK-NEXT: (ref.get_desc $used + ;; CHECK-NEXT: (local.get $used) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $use (param $used (ref $used)) (result (ref $used.desc)) + (ref.get_desc $used + (local.get $used) + ) + ) +) + +;; Now the public supertype does not have a descriptor, so we can still +;; optimize. +(module + (rec + ;; CHECK: (type $public (sub (struct))) + (type $public (sub (struct))) + ) + (rec + ;; CHECK: (rec + ;; CHECK-NEXT: (type $1 (descriptor $unused.desc (struct))) + + ;; CHECK: (type $unused (sub $public (struct))) + (type $unused (sub $public (descriptor $unused.desc (struct)))) + ;; CHECK: (type $unused.desc (sub (describes $1 (struct)))) + (type $unused.desc (sub (describes $unused (struct)))) + + ;; CHECK: (type $used (sub $unused (descriptor $used.desc (struct)))) + (type $used (sub $unused (descriptor $used.desc (struct)))) + ;; CHECK: (type $used.desc (sub $unused.desc (describes $used (struct)))) + (type $used.desc (sub $unused.desc (describes $used (struct)))) + ) + + ;; CHECK: (type $6 (func (param (ref $used)) (result (ref $used.desc)))) + + ;; CHECK: (global $public (ref null $public) (ref.null none)) + (global $public (export "public") (ref null $public) (ref.null none)) + + ;; CHECK: (export "public" (global $public)) + + ;; CHECK: (func $use (type $6) (param $used (ref $used)) (result (ref $used.desc)) + ;; CHECK-NEXT: (ref.get_desc $used + ;; CHECK-NEXT: (local.get $used) + ;; CHECK-NEXT: ) + ;; CHECK-NEXT: ) + (func $use (param $used (ref $used)) (result (ref $used.desc)) + (ref.get_desc $used + (local.get $used) + ) + ) +) + ;; Sibling types $A and $B. The supertype that connects them should not stop us ;; from optimizing $B here, even though $A cannot be optimized. (module