Skip to content

Commit d5157e0

Browse files
kripkenkripken
authored
SubtypingDiscoverer: Differentiate non-flow subtyping constraints (#6344)
When we do a local.set of a value into a local then we have both a subtyping constraint - for the value to be valid to put in that local - and also a flow of a value, which can then reach more places. Such flow then interacts with casts in Unsubtyping, since it needs to know what can flow where in order to know how casts force us to keep subtyping relations. That regressed in the not-actually-NFC #6323 in which I added the innocuous lines to add subtyping constraints in ref.eq. It seems fine to require that the arms of a RefEq must be of type eqref, but Unsubtyping then assuming those arms flowed into a location of type eqref... which means casts might force us to not optimize some things. To fix this, differentiate the rare case of non-flowing subtyping constraints, which is basically only RefEq. There are perhaps a few more cases (like i31 operations) but they do not matter in practice for Unsubtyping anyhow; I suggest we land this first to undo the regression and then at our leisure investigate the other instructions.
1 parent f868137 commit d5157e0

File tree

4 files changed

+141
-2
lines changed

4 files changed

+141
-2
lines changed

‎src/ir/subtype-exprs.h

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,13 +59,29 @@ namespace wasm {
5959
// * noteCast(Expression, Expression) - An expression's type is cast to
6060
// another, for example, in RefCast.
6161
//
62+
// In addition, we need to differentiate two situations that cause subtyping:
63+
// * Flow-based subtyping: E.g. when a value flows out from a block, in which
64+
// case the value must be a subtype of the block's type.
65+
// * Non-flow-based subtyping: E.g. in RefEq, being in one of the arms means
66+
// you must be a subtype of eqref, but your value does not flow anywhere,
67+
// because it is processed by the RefEq and does not send it anywhere.
68+
// The difference between the two matters in some users of this class, and so
69+
// the above functions all handle flow-based subtyping, while there is also the
70+
// following:
71+
//
72+
// * noteNonFlowSubtype(Expression, Type)
73+
//
74+
// This is the only signature we need for the non-flowing case since it always
75+
// stems from an expression that is compared against a type.
76+
//
6277
// The concrete signatures are:
6378
//
6479
// void noteSubtype(Type, Type);
6580
// void noteSubtype(HeapType, HeapType);
6681
// void noteSubtype(Type, Expression*);
6782
// void noteSubtype(Expression*, Type);
6883
// void noteSubtype(Expression*, Expression*);
84+
// void noteNonFlowSubtype(Expression*, Type);
6985
// void noteCast(HeapType, HeapType);
7086
// void noteCast(Expression*, Type);
7187
// void noteCast(Expression*, Expression*);
@@ -202,8 +218,8 @@ struct SubtypingDiscoverer : public OverriddenVisitor<SubType> {
202218
void visitRefIsNull(RefIsNull* curr) {}
203219
void visitRefFunc(RefFunc* curr) {}
204220
void visitRefEq(RefEq* curr) {
205-
self()->noteSubtype(curr->left, Type(HeapType::eq, Nullable));
206-
self()->noteSubtype(curr->right, Type(HeapType::eq, Nullable));
221+
self()->noteNonFlowSubtype(curr->left, Type(HeapType::eq, Nullable));
222+
self()->noteNonFlowSubtype(curr->right, Type(HeapType::eq, Nullable));
207223
}
208224
void visitTableGet(TableGet* curr) {}
209225
void visitTableSet(TableSet* curr) {

‎src/passes/StringLowering.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,10 @@ struct StringLowering : public StringGathering {
478478
// Only the type matters of the place we assign to.
479479
noteSubtype(a, b->type);
480480
}
481+
void noteNonFlowSubtype(Expression* a, Type b) {
482+
// Flow or non-flow is the same for us.
483+
noteSubtype(a, b);
484+
}
481485
void noteCast(HeapType, HeapType) {
482486
// Casts do not concern us.
483487
}

‎src/passes/Unsubtyping.cpp

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,22 @@ struct Unsubtyping
193193
noteSubtype(sub->type, super->type);
194194
}
195195

196+
void noteNonFlowSubtype(Expression* sub, Type super) {
197+
// This expression's type must be a subtype of |super|, but the value does
198+
// not flow anywhere - this is a static constraint. As the value does not
199+
// flow, it cannot reach anywhere else, which means we need this in order to
200+
// validate but it does not interact with casts. Given that, if super is a
201+
// basic type then we can simply ignore this: we only remove subtyping
202+
// between user types, so subtyping wrt basic types is unchanged, and so
203+
// this constraint will never be a problem.
204+
if (super.isRef() && super.getHeapType().isBasic()) {
205+
return;
206+
}
207+
208+
// Otherwise, we must take this into account.
209+
noteSubtype(sub, super);
210+
}
211+
196212
void noteCast(HeapType src, HeapType dest) {
197213
if (src == dest || dest.isBottom()) {
198214
return;

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,109 @@
372372
)
373373
)
374374

375+
;; As above, but now with some ref.eq added. Those should not inhibit
376+
;; optimizations: as before, $bot no longer needs to subtype from $mid (but
377+
;; $mid must subtype from $top). ref.eq does add a requirement on subtyping
378+
;; (that the type be a subtype of eq), but ref.eq does not actually flow the
379+
;; inputs it receives anywhere, so that doesn't stop us from removing subtyping
380+
;; from user types.
381+
(module
382+
(rec
383+
;; CHECK: (rec
384+
;; CHECK-NEXT: (type $top (sub (struct )))
385+
(type $top (sub (struct)))
386+
;; CHECK: (type $mid (sub $top (struct )))
387+
(type $mid (sub $top (struct)))
388+
;; CHECK: (type $bot (sub (struct )))
389+
(type $bot (sub $mid (struct)))
390+
)
391+
392+
;; CHECK: (type $3 (func (param anyref (ref $top) (ref $mid) (ref $bot))))
393+
394+
;; CHECK: (func $cast (type $3) (param $any anyref) (param $top (ref $top)) (param $mid (ref $mid)) (param $bot (ref $bot))
395+
;; CHECK-NEXT: (local $l anyref)
396+
;; CHECK-NEXT: (drop
397+
;; CHECK-NEXT: (ref.eq
398+
;; CHECK-NEXT: (local.get $top)
399+
;; CHECK-NEXT: (local.get $mid)
400+
;; CHECK-NEXT: )
401+
;; CHECK-NEXT: )
402+
;; CHECK-NEXT: (drop
403+
;; CHECK-NEXT: (ref.eq
404+
;; CHECK-NEXT: (local.get $top)
405+
;; CHECK-NEXT: (local.get $bot)
406+
;; CHECK-NEXT: )
407+
;; CHECK-NEXT: )
408+
;; CHECK-NEXT: (drop
409+
;; CHECK-NEXT: (ref.eq
410+
;; CHECK-NEXT: (local.get $mid)
411+
;; CHECK-NEXT: (local.get $bot)
412+
;; CHECK-NEXT: )
413+
;; CHECK-NEXT: )
414+
;; CHECK-NEXT: (drop
415+
;; CHECK-NEXT: (ref.cast (ref $bot)
416+
;; CHECK-NEXT: (local.get $any)
417+
;; CHECK-NEXT: )
418+
;; CHECK-NEXT: )
419+
;; CHECK-NEXT: (drop
420+
;; CHECK-NEXT: (ref.cast (ref $top)
421+
;; CHECK-NEXT: (local.get $any)
422+
;; CHECK-NEXT: )
423+
;; CHECK-NEXT: )
424+
;; CHECK-NEXT: (drop
425+
;; CHECK-NEXT: (ref.cast (ref $mid)
426+
;; CHECK-NEXT: (local.get $any)
427+
;; CHECK-NEXT: )
428+
;; CHECK-NEXT: )
429+
;; CHECK-NEXT: (local.set $l
430+
;; CHECK-NEXT: (struct.new_default $mid)
431+
;; CHECK-NEXT: )
432+
;; CHECK-NEXT: )
433+
(func $cast (param $any anyref) (param $top (ref $top)) (param $mid (ref $mid)) (param $bot (ref $bot))
434+
(local $l anyref)
435+
(drop
436+
(ref.eq
437+
(local.get $top)
438+
(local.get $mid)
439+
)
440+
)
441+
(drop
442+
(ref.eq
443+
(local.get $top)
444+
(local.get $bot)
445+
)
446+
)
447+
(drop
448+
(ref.eq
449+
(local.get $mid)
450+
(local.get $bot)
451+
)
452+
)
453+
(drop
454+
;; Cast any -> $bot
455+
(ref.cast (ref $bot)
456+
(local.get $any)
457+
)
458+
)
459+
(drop
460+
;; Cast any -> $top
461+
(ref.cast (ref $top)
462+
(local.get $any)
463+
)
464+
)
465+
(drop
466+
;; Cast any -> $mid
467+
(ref.cast (ref $mid)
468+
(local.get $any)
469+
)
470+
)
471+
472+
(local.set $l
473+
(struct.new $mid)
474+
)
475+
)
476+
)
477+
375478
(module
376479
(rec
377480
;; CHECK: (rec

0 commit comments

Comments
 (0)