Skip to content

Commit 4ffe272

Browse files
authored
ReFinalize after merging siblings in TypeMerging (#7121)
The LUB of sibling types is their common supertype, but after the sibling types are merged, their LUB is the merged type, which is a strict subtype of the previous LUB. This means that merging sibling types causes `selects` to have stale types when the two select arms previously had the two merged sibling types. To fix any potential stale types, ReFinalize after merging sibling types.
1 parent ffc3f22 commit 4ffe272

2 files changed

Lines changed: 72 additions & 1 deletion

File tree

src/passes/TypeMerging.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
#include "ir/module-utils.h"
4040
#include "ir/type-updating.h"
41+
#include "ir/utils.h"
4142
#include "pass.h"
4243
#include "support/dfa_minimization.h"
4344
#include "support/small_set.h"
@@ -229,14 +230,24 @@ void TypeMerging::run(Module* module_) {
229230
// Merging can unlock more sibling merging opportunities because two identical
230231
// types cannot be merged until their respective identical parents have been
231232
// merged in a previous step, making them siblings.
233+
//
234+
// If we merge siblings, we also need to refinalize because the LUB of merged
235+
// siblings is the merged type rather than their common supertype after the
236+
// merge.
237+
bool refinalize = false;
232238
merge(Supertypes);
233239
for (int i = 0; i < MAX_ITERATIONS; ++i) {
234240
if (!merge(Siblings)) {
235241
break;
236242
}
243+
refinalize = true;
237244
}
238245

239246
applyMerges();
247+
248+
if (refinalize) {
249+
ReFinalize().run(getPassRunner(), module);
250+
}
240251
}
241252

242253
bool TypeMerging::merge(MergeKind kind) {

test/lit/passes/type-merging.wast

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,6 @@
877877
;; leading to a failure to build new types.
878878
;; TODO: Store a heap type on control flow structures to avoid creating
879879
;; standalone function types for them.
880-
;; TODO: Investigate why the rec group contains two of the same type below.
881880
(module
882881
(rec
883882
;; CHECK: (rec
@@ -987,6 +986,67 @@
987986
(export "public" (global $public))
988987
)
989988

989+
;; Regression test that produces invalid IR if we do not refinalize after
990+
;; merging siblings.
991+
(module
992+
(rec
993+
;; CHECK: (rec
994+
;; CHECK-NEXT: (type $super (sub (struct)))
995+
(type $super (sub (struct)))
996+
;; CHECK: (type $subA (sub $super (struct (field i32))))
997+
(type $subA (sub $super (struct (field i32))))
998+
(type $subB (sub $super (struct (field i32))))
999+
)
1000+
1001+
;; CHECK: (type $2 (func))
1002+
1003+
;; CHECK: (func $test (type $2)
1004+
;; CHECK-NEXT: (drop
1005+
;; CHECK-NEXT: (select (result (ref $subA))
1006+
;; CHECK-NEXT: (struct.new_default $subA)
1007+
;; CHECK-NEXT: (struct.new_default $subA)
1008+
;; CHECK-NEXT: (i32.const 0)
1009+
;; CHECK-NEXT: )
1010+
;; CHECK-NEXT: )
1011+
;; CHECK-NEXT: )
1012+
(func $test
1013+
(drop
1014+
(select (result (ref $super))
1015+
(struct.new_default $subA)
1016+
(struct.new_default $subB)
1017+
(i32.const 0)
1018+
)
1019+
)
1020+
)
1021+
)
1022+
1023+
;; TODO: We should be able to merge $sub into $public here, but we currently do
1024+
;; not encode the successors of public types in their DFA states, so we
1025+
;; currently fail to do this merge. See issue #7120.
1026+
(module
1027+
;; CHECK: (type $public (sub (struct (field (ref null $public)))))
1028+
(type $public (sub (struct (field (ref null $public)))))
1029+
;; CHECK: (rec
1030+
;; CHECK-NEXT: (type $sub (sub $public (struct (field (ref null $public)))))
1031+
(type $sub (sub $public (struct (field (ref null $public)))))
1032+
1033+
;; CHECK: (type $2 (func))
1034+
1035+
;; CHECK: (global $g (ref null $public) (ref.null none))
1036+
(global $g (export "g") (ref null $public) (ref.null none))
1037+
1038+
;; CHECK: (export "g" (global $g))
1039+
1040+
;; CHECK: (func $test (type $2)
1041+
;; CHECK-NEXT: (local $0 (ref $public))
1042+
;; CHECK-NEXT: (local $1 (ref $sub))
1043+
;; CHECK-NEXT: )
1044+
(func $test
1045+
(local (ref $public))
1046+
(local (ref $sub))
1047+
)
1048+
)
1049+
9901050
;; Check that a ref.test inhibits merging (ref.cast is already checked above).
9911051
(module
9921052
;; CHECK: (rec

0 commit comments

Comments
 (0)