Skip to content

Commit 9fe0d16

Browse files
tlivelytlively
authored
[NFC] Update ChildTyper callback for unknown constaints (WebAssembly#7814)
When ChildTyper does not have enough type information to generate type constraints (for example, because a child whose type provides crucial information is unreachable), it previously allowed the user to determine whether to return early or not. But not returning early would then lead to an assertion failure or other bad behavior, so it is never a reasonable choice. The only real choice a user has is to return early and ignore the situation (which is what TypeSSA wants) or to fail loudly because this should never happen (which is what IRBuilder wants). Simplify the callback to allow customizing this behavior but not whether the early return ocurrs.
1 parent 425cd4d commit 9fe0d16

File tree

3 files changed

+77
-58
lines changed

3 files changed

+77
-58
lines changed

‎src/ir/child-typer.h‎

Lines changed: 70 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,9 @@ namespace wasm {
6666
// two non-reference children of `select` must have the same type because that
6767
// would require inspecting the types of those children.
6868
//
69-
// The skipUnreachable() hook function determines the behavior on code that
70-
// we cannot process due to unreachability. For example, an unreachable
71-
// StructNew has no struct type defined, so we cannot apply its heap type. By
72-
// default we do not skip such unreachable code, and error in such such cases if
73-
// the type is not provided (by passing the heap type as mentioned above, as a
74-
// parameter to the visit* method). Optimization passes can often skip
75-
// unreachable code (leaving it for DCE), while other operations might not.
69+
// The noteUnknown() hook is called whenever there is insufficient type
70+
// information to generate constraints for all the children. Some users may wish
71+
// to ignore this situation and others may want to assert that it never occurs.
7672
template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
7773
Module& wasm;
7874
Function* func;
@@ -105,8 +101,6 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
105101

106102
Type getLabelType(Name label) { return self().getLabelType(label); }
107103

108-
bool skipUnreachable() { return false; }
109-
110104
void visitNop(Nop* curr) {}
111105

112106
void visitBlock(Block* curr) {
@@ -195,7 +189,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
195189
}
196190

197191
void visitAtomicRMW(AtomicRMW* curr) {
198-
if (self().skipUnreachable() && curr->type == Type::unreachable) {
192+
if (curr->type == Type::unreachable) {
193+
self().noteUnknown();
199194
return;
200195
}
201196
assert(curr->type == Type::i32 || curr->type == Type::i64);
@@ -828,7 +823,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
828823
void visitTupleExtract(TupleExtract* curr,
829824
std::optional<size_t> arity = std::nullopt) {
830825
if (!arity) {
831-
if (self().skipUnreachable() && !curr->tuple->type.isTuple()) {
826+
if (!curr->tuple->type.isTuple()) {
827+
self().noteUnknown();
832828
return;
833829
}
834830
assert(curr->tuple->type.isTuple());
@@ -845,7 +841,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
845841

846842
void visitCallRef(CallRef* curr, std::optional<HeapType> ht = std::nullopt) {
847843
if (!ht) {
848-
if (self().skipUnreachable() && !curr->target->type.isRef()) {
844+
if (!curr->target->type.isRef()) {
845+
self().noteUnknown();
849846
return;
850847
}
851848
ht = curr->target->type.getHeapType().getSignature();
@@ -859,36 +856,32 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
859856
}
860857

861858
void visitRefTest(RefTest* curr) {
862-
if (self().skipUnreachable() && !curr->castType.isRef()) {
863-
return;
864-
}
865859
auto top = curr->castType.getHeapType().getTop();
866860
note(&curr->ref, Type(top, Nullable));
867861
}
868862

869863
void visitRefCast(RefCast* curr, std::optional<Type> target = std::nullopt) {
870-
if (self().skipUnreachable() && !curr->type.isRef()) {
871-
return;
864+
if (!target) {
865+
if (!curr->type.isRef()) {
866+
self().noteUnknown();
867+
return;
868+
}
869+
target = curr->type;
872870
}
873-
auto top = curr->type.getHeapType().getTop();
871+
auto top = target->getHeapType().getTop();
874872
note(&curr->ref, Type(top, Nullable));
875873
if (curr->desc) {
876-
if (!target) {
877-
target = curr->type;
878-
}
879-
if (self().skipUnreachable() && !target->isRef()) {
880-
return;
881-
}
882874
auto desc = target->getHeapType().getDescriptorType();
883875
assert(desc);
884-
note(&curr->desc, Type(*desc, Nullable, curr->type.getExactness()));
876+
note(&curr->desc, Type(*desc, Nullable, target->getExactness()));
885877
}
886878
}
887879

888880
void visitRefGetDesc(RefGetDesc* curr,
889881
std::optional<HeapType> ht = std::nullopt) {
890882
if (!ht) {
891-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
883+
if (!curr->ref->type.isRef()) {
884+
self().noteUnknown();
892885
return;
893886
}
894887
ht = curr->ref->type.getHeapType();
@@ -900,18 +893,19 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
900893
switch (curr->op) {
901894
case BrOnNull:
902895
case BrOnNonNull:
896+
// br_on(_non)_null is polymorphic over reference types and does not
897+
// take a type immediate.
898+
assert(!target);
903899
noteAnyReference(&curr->ref);
904900
return;
905901
case BrOnCast:
906902
case BrOnCastFail:
907903
case BrOnCastDesc:
908904
case BrOnCastDescFail: {
909905
if (!target) {
906+
assert(curr->castType.isRef());
910907
target = curr->castType;
911908
}
912-
if (self().skipUnreachable() && !target->isRef()) {
913-
return;
914-
}
915909
auto top = target->getHeapType().getTop();
916910
note(&curr->ref, Type(top, Nullable));
917911
if (curr->op == BrOnCastDesc || curr->op == BrOnCastDescFail) {
@@ -926,7 +920,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
926920
}
927921

928922
void visitStructNew(StructNew* curr) {
929-
if (self().skipUnreachable() && !curr->type.isRef()) {
923+
if (!curr->type.isRef()) {
924+
self().noteUnknown();
930925
return;
931926
}
932927
if (!curr->isWithDefault()) {
@@ -945,7 +940,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
945940
void visitStructGet(StructGet* curr,
946941
std::optional<HeapType> ht = std::nullopt) {
947942
if (!ht) {
948-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
943+
if (!curr->ref->type.isRef()) {
944+
self().noteUnknown();
949945
return;
950946
}
951947
ht = curr->ref->type.getHeapType();
@@ -956,7 +952,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
956952
void visitStructSet(StructSet* curr,
957953
std::optional<HeapType> ht = std::nullopt) {
958954
if (!ht) {
959-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
955+
if (!curr->ref->type.isRef()) {
956+
self().noteUnknown();
960957
return;
961958
}
962959
ht = curr->ref->type.getHeapType();
@@ -970,7 +967,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
970967
void visitStructRMW(StructRMW* curr,
971968
std::optional<HeapType> ht = std::nullopt) {
972969
if (!ht) {
973-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
970+
if (!curr->ref->type.isRef()) {
971+
self().noteUnknown();
974972
return;
975973
}
976974
ht = curr->ref->type.getHeapType();
@@ -984,7 +982,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
984982
void visitStructCmpxchg(StructCmpxchg* curr,
985983
std::optional<HeapType> ht = std::nullopt) {
986984
if (!ht) {
987-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
985+
if (!curr->ref->type.isRef()) {
986+
self().noteUnknown();
988987
return;
989988
}
990989
ht = curr->ref->type.getHeapType();
@@ -999,7 +998,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
999998

1000999
void visitArrayNew(ArrayNew* curr) {
10011000
if (!curr->isWithDefault()) {
1002-
if (self().skipUnreachable() && !curr->type.isRef()) {
1001+
if (!curr->type.isRef()) {
1002+
self().noteUnknown();
10031003
return;
10041004
}
10051005
note(&curr->init, curr->type.getHeapType().getArray().element.type);
@@ -1018,7 +1018,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10181018
}
10191019

10201020
void visitArrayNewFixed(ArrayNewFixed* curr) {
1021-
if (self().skipUnreachable() && !curr->type.isRef()) {
1021+
if (!curr->type.isRef()) {
1022+
self().noteUnknown();
10221023
return;
10231024
}
10241025
auto type = curr->type.getHeapType().getArray().element.type;
@@ -1030,7 +1031,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10301031
void visitArrayGet(ArrayGet* curr,
10311032
std::optional<HeapType> ht = std::nullopt) {
10321033
if (!ht) {
1033-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1034+
if (!curr->ref->type.isRef()) {
1035+
self().noteUnknown();
10341036
return;
10351037
}
10361038
ht = curr->ref->type.getHeapType();
@@ -1042,7 +1044,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10421044
void visitArraySet(ArraySet* curr,
10431045
std::optional<HeapType> ht = std::nullopt) {
10441046
if (!ht) {
1045-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1047+
if (!curr->ref->type.isRef()) {
1048+
self().noteUnknown();
10461049
return;
10471050
}
10481051
ht = curr->ref->type.getHeapType();
@@ -1061,13 +1064,15 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10611064
std::optional<HeapType> dest = std::nullopt,
10621065
std::optional<HeapType> src = std::nullopt) {
10631066
if (!dest) {
1064-
if (self().skipUnreachable() && !curr->destRef->type.isRef()) {
1067+
if (!curr->destRef->type.isRef()) {
1068+
self().noteUnknown();
10651069
return;
10661070
}
10671071
dest = curr->destRef->type.getHeapType();
10681072
}
10691073
if (!src) {
1070-
if (self().skipUnreachable() && !curr->srcRef->type.isRef()) {
1074+
if (!curr->srcRef->type.isRef()) {
1075+
self().noteUnknown();
10711076
return;
10721077
}
10731078
src = curr->srcRef->type.getHeapType();
@@ -1082,7 +1087,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10821087
void visitArrayFill(ArrayFill* curr,
10831088
std::optional<HeapType> ht = std::nullopt) {
10841089
if (!ht) {
1085-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1090+
if (!curr->ref->type.isRef()) {
1091+
self().noteUnknown();
10861092
return;
10871093
}
10881094
ht = curr->ref->type.getHeapType();
@@ -1097,7 +1103,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
10971103
void visitArrayInitData(ArrayInitData* curr,
10981104
std::optional<HeapType> ht = std::nullopt) {
10991105
if (!ht) {
1100-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1106+
if (!curr->ref->type.isRef()) {
1107+
self().noteUnknown();
11011108
return;
11021109
}
11031110
ht = curr->ref->type.getHeapType();
@@ -1111,7 +1118,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11111118
void visitArrayInitElem(ArrayInitElem* curr,
11121119
std::optional<HeapType> ht = std::nullopt) {
11131120
if (!ht) {
1114-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1121+
if (!curr->ref->type.isRef()) {
1122+
self().noteUnknown();
11151123
return;
11161124
}
11171125
ht = curr->ref->type.getHeapType();
@@ -1125,7 +1133,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11251133
void visitArrayRMW(ArrayRMW* curr,
11261134
std::optional<HeapType> ht = std::nullopt) {
11271135
if (!ht) {
1128-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1136+
if (!curr->ref->type.isRef()) {
1137+
self().noteUnknown();
11291138
return;
11301139
}
11311140
ht = curr->ref->type.getHeapType();
@@ -1139,7 +1148,8 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
11391148
void visitArrayCmpxchg(ArrayCmpxchg* curr,
11401149
std::optional<HeapType> ht = std::nullopt) {
11411150
if (!ht) {
1142-
if (self().skipUnreachable() && !curr->ref->type.isRef()) {
1151+
if (!curr->ref->type.isRef()) {
1152+
self().noteUnknown();
11431153
return;
11441154
}
11451155
ht = curr->ref->type.getHeapType();
@@ -1241,14 +1251,16 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
12411251
void visitContBind(ContBind* curr,
12421252
std::optional<HeapType> src = std::nullopt,
12431253
std::optional<HeapType> dest = std::nullopt) {
1244-
if (!src.has_value()) {
1245-
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1254+
if (!src) {
1255+
if (!curr->cont->type.isRef()) {
1256+
self().noteUnknown();
12461257
return;
12471258
}
12481259
src = curr->cont->type.getHeapType();
12491260
}
1250-
if (!dest.has_value()) {
1251-
if (self().skipUnreachable() && !curr->type.isRef()) {
1261+
if (!dest) {
1262+
if (!curr->type.isRef()) {
1263+
self().noteUnknown();
12521264
return;
12531265
}
12541266
dest = curr->type.getHeapType();
@@ -1273,8 +1285,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
12731285
}
12741286

12751287
void visitResume(Resume* curr, std::optional<HeapType> ct = std::nullopt) {
1276-
if (!ct.has_value()) {
1277-
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1288+
if (!ct) {
1289+
if (!curr->cont->type.isRef()) {
1290+
self().noteUnknown();
12781291
return;
12791292
}
12801293
ct = curr->cont->type.getHeapType();
@@ -1290,8 +1303,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
12901303

12911304
void visitResumeThrow(ResumeThrow* curr,
12921305
std::optional<HeapType> ct = std::nullopt) {
1293-
if (!ct.has_value()) {
1294-
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1306+
if (!ct) {
1307+
if (!curr->cont->type.isRef()) {
1308+
self().noteUnknown();
12951309
return;
12961310
}
12971311
ct = curr->cont->type.getHeapType();
@@ -1307,8 +1321,9 @@ template<typename Subtype> struct ChildTyper : OverriddenVisitor<Subtype> {
13071321

13081322
void visitStackSwitch(StackSwitch* curr,
13091323
std::optional<HeapType> ct = std::nullopt) {
1310-
if (!ct.has_value()) {
1311-
if (self().skipUnreachable() && !curr->cont->type.isRef()) {
1324+
if (!ct) {
1325+
if (!curr->cont->type.isRef()) {
1326+
self().noteUnknown();
13121327
return;
13131328
}
13141329
ct = curr->cont->type.getHeapType();

‎src/passes/TypeSSA.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -238,9 +238,8 @@ struct Analyzer
238238

239239
Type getLabelType(Name label) { WASM_UNREACHABLE("unexpected branch"); }
240240

241-
// Skip unreachable code. If we cannot compute a constraint due to
242-
// unreachability, we can ignore it.
243-
bool skipUnreachable() { return true; }
241+
// We don't mind if we cannot compute a constraint due to unreachability.
242+
void noteUnknown() {}
244243
} typer(*this);
245244
typer.visit(curr);
246245
}

‎src/wasm/wasm-ir-builder.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,11 @@ struct IRBuilder::ChildPopper
401401
// condition.
402402
children.push_back({&curr->condition, {Subtype{Type::i32}}});
403403
}
404+
405+
// It is a bug if we ever have insufficient type information.
406+
void noteUnknown() {
407+
WASM_UNREACHABLE("unexpected insufficient type information");
408+
}
404409
};
405410

406411
IRBuilder& builder;

0 commit comments

Comments
 (0)