Skip to content

Commit 9be5515

Browse files
tlivelytlively
committed
Merge branch 'unsubtyping-optimize-descs' into unsubtyping-exact-casts
2 parents 2e2fa27 + 6559430 commit 9be5515

4 files changed

Lines changed: 119 additions & 17 deletions

File tree

‎scripts/fuzz_opt.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1536,7 +1536,7 @@ def can_run_on_wasm(self, wasm):
15361536
# files, which adds coverage for ClusterFuzz (which sometimes runs two wasm
15371537
# files in that way).
15381538
class Split(TestCaseHandler):
1539-
frequency = 1 # TODO: adjust lower when we actually enable this
1539+
frequency = 0.1
15401540

15411541
def handle(self, wasm):
15421542
# get the list of function names, some of which we will decide to split
@@ -2121,8 +2121,7 @@ def handle(self, wasm):
21212121
TrapsNeverHappen(),
21222122
CtorEval(),
21232123
Merge(),
2124-
# TODO: enable when stable enough, and adjust |frequency| (see above)
2125-
# Split(),
2124+
Split(),
21262125
RoundtripText(),
21272126
ClusterFuzz(),
21282127
Two(),

‎src/passes/Unsubtyping.cpp‎

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,10 @@
2525
#include <unordered_set>
2626
#endif
2727

28+
#include "ir/effects.h"
2829
#include "ir/localize.h"
2930
#include "ir/module-utils.h"
31+
#include "ir/names.h"
3032
#include "ir/subtype-exprs.h"
3133
#include "ir/type-updating.h"
3234
#include "ir/utils.h"
@@ -675,10 +677,15 @@ struct Unsubtyping : Pass {
675677
// and outside a function context and we assume it may be null and cause
676678
// a trap, then we have no way to preserve that trap without keeping the
677679
// descriptor around.
678-
if (!trapsNeverHappen && !getFunction() &&
679-
curr->desc->type.isNullable()) {
680-
noteDescribed(curr->type.getHeapType());
680+
if (trapsNeverHappen || getFunction() ||
681+
curr->desc->type.isNonNullable()) {
682+
return;
681683
}
684+
// We must preserve the potential trap. When we update the instructions
685+
// later we will move this allocation to a new global if necessary to
686+
// preserve the potential trap even if a parent of the current
687+
// expression is removed.
688+
noteDescribed(curr->type.getHeapType());
682689
}
683690
};
684691

@@ -927,6 +934,11 @@ struct Unsubtyping : Pass {
927934
struct Rewriter : WalkerPass<PostWalker<Rewriter>> {
928935
const TypeTree& types;
929936

937+
// Allocations that might trap that have been removed from module-level
938+
// initializers. These need to be placed in new globals to preserve any
939+
// instantiation-time traps.
940+
std::vector<Expression*> removedTrappingInits;
941+
930942
Rewriter(const TypeTree& types) : types(types) {}
931943

932944
bool isFunctionParallel() override { return true; }
@@ -962,15 +974,32 @@ struct Unsubtyping : Pass {
962974
block->list.push_back(curr);
963975
block->type = curr->type;
964976
replaceCurrent(block);
977+
} else {
978+
// We are dropping this descriptor, but it might have a potential trap
979+
// nested inside it. In that case we need to preserve the trap by
980+
// moving this descriptor to a new global.
981+
if (curr->desc->is<StructNew>() &&
982+
EffectAnalyzer(getPassOptions(), *getModule(), curr->desc).trap) {
983+
removedTrappingInits.push_back(curr->desc);
984+
}
965985
}
966986
curr->desc = nullptr;
967987
}
968988
};
969989

970-
PassRunner runner(getPassRunner());
971-
runner.add(std::make_unique<Rewriter>(types));
972-
runner.run();
973-
Rewriter(types).runOnModuleCode(getPassRunner(), &wasm);
990+
Rewriter rewriter(types);
991+
rewriter.run(getPassRunner(), &wasm);
992+
rewriter.runOnModuleCode(getPassRunner(), &wasm);
993+
994+
// Insert globals necessary to preserve instantiation-time trapping of
995+
// removed allocations.
996+
for (Index i = 0; i < rewriter.removedTrappingInits.size(); ++i) {
997+
auto* curr = rewriter.removedTrappingInits[i];
998+
auto name = Names::getValidGlobalName(
999+
wasm, std::string("unsubtyping-removed-") + std::to_string(i));
1000+
wasm.addGlobal(
1001+
Builder::makeGlobal(name, curr->type, curr, Builder::Immutable));
1002+
}
9741003
}
9751004
};
9761005

‎test/lit/passes/unsubtyping-desc-tnh.wast‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,3 +47,35 @@
4747
)
4848
)
4949
)
50+
51+
;; Nested allocations do not need to be moved to new globals when traps never
52+
;; happen.
53+
(module
54+
(rec
55+
;; CHECK: (type $struct (sub (struct)))
56+
(type $struct (sub (descriptor $desc (struct))))
57+
(type $desc (sub (describes $struct (descriptor $meta (struct)))))
58+
(type $meta (sub (describes $desc (struct))))
59+
)
60+
61+
;; CHECK: (global $g (ref $struct) (struct.new_default $struct))
62+
(global $g (ref $struct) (struct.new $struct (struct.new $desc (ref.null none))))
63+
)
64+
65+
;; Same, but now the nesting is under a non-descriptor field.
66+
(module
67+
(rec
68+
;; CHECK: (rec
69+
;; CHECK-NEXT: (type $A (sub (struct (field (ref $struct)))))
70+
(type $A (sub (struct (field (ref $struct)))))
71+
;; CHECK: (type $struct (sub (struct)))
72+
(type $struct (sub (descriptor $desc (struct))))
73+
(type $desc (sub (describes $struct (descriptor $meta (struct)))))
74+
(type $meta (sub (describes $desc (struct))))
75+
)
76+
77+
;; CHECK: (global $g (ref $A) (struct.new $A
78+
;; CHECK-NEXT: (struct.new_default $struct)
79+
;; CHECK-NEXT: ))
80+
(global $g (ref $A) (struct.new $A (struct.new $struct (struct.new $desc (ref.null none)))))
81+
)

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

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@
5858

5959
;; CHECK: (type $2 (func (param (ref null (exact $A.desc)))))
6060

61-
;; CHECK: (func $nullable-descs (type $2) (param $A.desc (ref null (exact $A.desc)))
61+
;; CHECK: (func $nullable-desc (type $2) (param $A.desc (ref null (exact $A.desc)))
6262
;; CHECK-NEXT: (local $1 (ref (exact $A.desc)))
6363
;; CHECK-NEXT: (drop
6464
;; CHECK-NEXT: (block (result (ref (exact $A)))
@@ -71,7 +71,7 @@
7171
;; CHECK-NEXT: )
7272
;; CHECK-NEXT: )
7373
;; CHECK-NEXT: )
74-
(func $nullable-descs (param $A.desc (ref null (exact $A.desc)))
74+
(func $nullable-desc (param $A.desc (ref null (exact $A.desc)))
7575
(drop
7676
(struct.new $A
7777
(local.get $A.desc)
@@ -89,17 +89,16 @@
8989
;; CHECK: (type $A.desc (sub (struct)))
9090
(type $A.desc (sub (describes $A (struct))))
9191
)
92-
9392
;; CHECK: (type $2 (func (param (ref (exact $A.desc)))))
9493

95-
;; CHECK: (func $nullable-descs (type $2) (param $A.desc (ref (exact $A.desc)))
94+
;; CHECK: (func $nonnullable-desc (type $2) (param $A.desc (ref (exact $A.desc)))
9695
;; CHECK-NEXT: (drop
9796
;; CHECK-NEXT: (block (result (ref (exact $A)))
9897
;; CHECK-NEXT: (struct.new_default $A)
9998
;; CHECK-NEXT: )
10099
;; CHECK-NEXT: )
101100
;; CHECK-NEXT: )
102-
(func $nullable-descs (param $A.desc (ref (exact $A.desc)))
101+
(func $nonnullable-desc (param $A.desc (ref (exact $A.desc)))
103102
(drop
104103
;; Now the descriptor is non-null.
105104
(struct.new $A
@@ -346,7 +345,7 @@
346345

347346
;; Now we still require B.desc <: A.desc, but now it is B.desc we require to
348347
;; remain a descriptor. This still requires A <: B and for A.desc to remain a
349-
;; descriptor as well, so we cannot optimize
348+
;; descriptor as well, so we cannot optimize.
350349
(module
351350
(rec
352351
;; CHECK: (rec
@@ -1166,9 +1165,52 @@
11661165
)
11671166
)
11681167

1168+
;; When the possibly-trapping global allocations are nested inside other
1169+
;; allocations that will be removed, they need to be moved to new globals.
1170+
(module
1171+
(rec
1172+
;; CHECK: (rec
1173+
;; CHECK-NEXT: (type $struct (sub (struct)))
1174+
(type $struct (sub (descriptor $desc (struct))))
1175+
;; CHECK: (type $desc (sub (descriptor $meta (struct))))
1176+
(type $desc (sub (describes $struct (descriptor $meta (struct)))))
1177+
;; CHECK: (type $meta (sub (describes $desc (struct))))
1178+
(type $meta (sub (describes $desc (struct))))
1179+
)
1180+
1181+
;; CHECK: (global $g (ref $struct) (struct.new_default $struct))
1182+
(global $g (ref $struct) (struct.new $struct (struct.new $desc (ref.null none))))
1183+
)
1184+
1185+
;; CHECK: (global $unsubtyping-removed-0 (ref (exact $desc)) (struct.new_default $desc
1186+
;; CHECK-NEXT: (ref.null none)
1187+
;; CHECK-NEXT: ))
1188+
(module
1189+
;; Same, but now the nesting is under a non-descriptor field.
1190+
(rec
1191+
;; CHECK: (rec
1192+
;; CHECK-NEXT: (type $A (sub (struct (field (ref $struct)))))
1193+
(type $A (sub (struct (field (ref $struct)))))
1194+
;; CHECK: (type $struct (sub (struct)))
1195+
(type $struct (sub (descriptor $desc (struct))))
1196+
;; CHECK: (type $desc (sub (descriptor $meta (struct))))
1197+
(type $desc (sub (describes $struct (descriptor $meta (struct)))))
1198+
;; CHECK: (type $meta (sub (describes $desc (struct))))
1199+
(type $meta (sub (describes $desc (struct))))
1200+
)
1201+
1202+
;; CHECK: (global $g (ref $A) (struct.new $A
1203+
;; CHECK-NEXT: (struct.new_default $struct)
1204+
;; CHECK-NEXT: ))
1205+
(global $g (ref $A) (struct.new $A (struct.new $struct (struct.new $desc (ref.null none)))))
1206+
)
1207+
1208+
;; CHECK: (global $unsubtyping-removed-0 (ref (exact $desc)) (struct.new_default $desc
1209+
;; CHECK-NEXT: (ref.null none)
1210+
;; CHECK-NEXT: ))
1211+
(module
11691212
;; This will be invalid soon, but in the meantime we should not be confused when
11701213
;; the types described by two related descriptors are unrelated.
1171-
(module
11721214
(rec
11731215
;; CHECK: (rec
11741216
;; CHECK-NEXT: (type $A (descriptor $super (struct)))

0 commit comments

Comments
 (0)