Skip to content

Commit 9704e14

Browse files
tlivelytlively
authored
[Custom Descriptors] Fix GTO descriptor optimization (#7897)
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.
1 parent 8b18494 commit 9704e14

File tree

2 files changed

+96
-6
lines changed

2 files changed

+96
-6
lines changed

‎src/passes/GlobalTypeOptimization.cpp‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -407,16 +407,22 @@ struct GlobalTypeOptimization : public Pass {
407407

408408
// Process the descriptor.
409409
if (auto desc = type.getDescriptorType()) {
410-
// To remove a descriptor, it must not be used in subtypes. It must also
411-
// have no write (see above, we note only dangerous writes which might
412-
// trap), as if it could trap, we'd have no easy way to remove it in a
413-
// global scope.
410+
// To remove a descriptor, it must not be read via supertypes and it
411+
// must not have to remain in supertypes because e.g. they are public.
412+
// It must also have no write (see above, we note only dangerous writes
413+
// which might trap), as if it could trap, we'd have no easy way to
414+
// remove it in a global scope.
414415
// TODO: We could check and handle the global scope specifically, but
415416
// the trapsNeverHappen flag avoids this problem entirely anyhow.
416417
//
417418
// This does not handle descriptor subtyping, see below.
418-
if (!dataFromSupers.desc.hasRead &&
419-
(!dataFromSupers.desc.hasWrite || trapsNeverHappen)) {
419+
auto super = type.getDeclaredSuperType();
420+
bool superNeedsDescriptor = super && super->getDescriptorType() &&
421+
!haveUnneededDescriptors.count(*super);
422+
bool descriptorIsUsed =
423+
dataFromSupers.desc.hasRead ||
424+
(dataFromSupers.desc.hasWrite && !trapsNeverHappen);
425+
if (!superNeedsDescriptor && !descriptorIsUsed) {
420426
haveUnneededDescriptors.insert(type);
421427
}
422428
}

‎test/lit/passes/gto-desc.wast‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,90 @@
10271027
)
10281028
)
10291029

1030+
;; Here we cannot optimize with placeholders because the public supertype needs
1031+
;; to keep its descriptor and its subtypes therefore need to keep their related
1032+
;; descriptors.
1033+
(module
1034+
(rec
1035+
;; CHECK: (rec
1036+
;; CHECK-NEXT: (type $public (sub (descriptor $public.desc (struct))))
1037+
(type $public (sub (descriptor $public.desc (struct))))
1038+
;; CHECK: (type $public.desc (sub (describes $public (struct))))
1039+
(type $public.desc (sub (describes $public (struct))))
1040+
)
1041+
(rec
1042+
;; CHECK: (rec
1043+
;; CHECK-NEXT: (type $unused (sub $public (descriptor $unused.desc (struct))))
1044+
(type $unused (sub $public (descriptor $unused.desc (struct))))
1045+
;; CHECK: (type $unused.desc (sub $public.desc (describes $unused (struct))))
1046+
(type $unused.desc (sub $public.desc (describes $unused (struct))))
1047+
1048+
;; CHECK: (type $used (sub $unused (descriptor $used.desc (struct))))
1049+
(type $used (sub $unused (descriptor $used.desc (struct))))
1050+
;; CHECK: (type $used.desc (sub $unused.desc (describes $used (struct))))
1051+
(type $used.desc (sub $unused.desc (describes $used (struct))))
1052+
)
1053+
1054+
;; CHECK: (type $6 (func (param (ref $used)) (result (ref $used.desc))))
1055+
1056+
;; CHECK: (global $public (ref null $public) (ref.null none))
1057+
(global $public (export "public") (ref null $public) (ref.null none))
1058+
1059+
;; CHECK: (export "public" (global $public))
1060+
1061+
;; CHECK: (func $use (type $6) (param $used (ref $used)) (result (ref $used.desc))
1062+
;; CHECK-NEXT: (ref.get_desc $used
1063+
;; CHECK-NEXT: (local.get $used)
1064+
;; CHECK-NEXT: )
1065+
;; CHECK-NEXT: )
1066+
(func $use (param $used (ref $used)) (result (ref $used.desc))
1067+
(ref.get_desc $used
1068+
(local.get $used)
1069+
)
1070+
)
1071+
)
1072+
1073+
;; Now the public supertype does not have a descriptor, so we can still
1074+
;; optimize.
1075+
(module
1076+
(rec
1077+
;; CHECK: (type $public (sub (struct)))
1078+
(type $public (sub (struct)))
1079+
)
1080+
(rec
1081+
;; CHECK: (rec
1082+
;; CHECK-NEXT: (type $1 (descriptor $unused.desc (struct)))
1083+
1084+
;; CHECK: (type $unused (sub $public (struct)))
1085+
(type $unused (sub $public (descriptor $unused.desc (struct))))
1086+
;; CHECK: (type $unused.desc (sub (describes $1 (struct))))
1087+
(type $unused.desc (sub (describes $unused (struct))))
1088+
1089+
;; CHECK: (type $used (sub $unused (descriptor $used.desc (struct))))
1090+
(type $used (sub $unused (descriptor $used.desc (struct))))
1091+
;; CHECK: (type $used.desc (sub $unused.desc (describes $used (struct))))
1092+
(type $used.desc (sub $unused.desc (describes $used (struct))))
1093+
)
1094+
1095+
;; CHECK: (type $6 (func (param (ref $used)) (result (ref $used.desc))))
1096+
1097+
;; CHECK: (global $public (ref null $public) (ref.null none))
1098+
(global $public (export "public") (ref null $public) (ref.null none))
1099+
1100+
;; CHECK: (export "public" (global $public))
1101+
1102+
;; CHECK: (func $use (type $6) (param $used (ref $used)) (result (ref $used.desc))
1103+
;; CHECK-NEXT: (ref.get_desc $used
1104+
;; CHECK-NEXT: (local.get $used)
1105+
;; CHECK-NEXT: )
1106+
;; CHECK-NEXT: )
1107+
(func $use (param $used (ref $used)) (result (ref $used.desc))
1108+
(ref.get_desc $used
1109+
(local.get $used)
1110+
)
1111+
)
1112+
)
1113+
10301114
;; Sibling types $A and $B. The supertype that connects them should not stop us
10311115
;; from optimizing $B here, even though $A cannot be optimized.
10321116
(module

0 commit comments

Comments
 (0)