Skip to content

Commit c4d15ef

Browse files
kripkenkripken
authored
Unreachability fixes for inlining (#5492)
We must refinalize as inlining unreachable code can lead to more things becoming unreachable. We also must uniquify label names before refinalizing, as the IR must be valid at that time, so that code is moved. This causes some minor changes to existing test code (some label changes, and refinalization makes more things unreachable), but only the two new tests show actual problems that needed to be fixed.
1 parent 8f98375 commit c4d15ef

File tree

5 files changed

+106
-36
lines changed

5 files changed

+106
-36
lines changed

‎src/passes/Inlining.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -413,6 +413,16 @@ static Expression* doInlining(Module* module,
413413
// Make the block reachable by adding a break to it
414414
block->list.push_back(builder.makeBreak(block->name));
415415
}
416+
// Anything we inlined into may now have non-unique label names, fix it up.
417+
// Note that we must do this before refinalization, as otherwise duplicate
418+
// block labels can lead to errors (the IR must be valid before we
419+
// refinalize).
420+
wasm::UniqueNameMapper::uniquify(into->body);
421+
// Inlining unreachable contents can make things in the function we inlined
422+
// into unreachable.
423+
ReFinalize().walkFunctionInModule(into, module);
424+
// New locals we added may require fixups for nondefaultability.
425+
// FIXME Is this not done automatically?
416426
TypeUpdating::handleNonDefaultableLocals(into, *module);
417427
return block;
418428
}
@@ -1039,11 +1049,6 @@ struct Inlining : public Pass {
10391049
assert(inlinedUses[inlinedName] <= infos[inlinedName].refs);
10401050
}
10411051
}
1042-
for (auto func : inlinedInto) {
1043-
// Anything we inlined into may now have non-unique label names, fix it
1044-
// up.
1045-
wasm::UniqueNameMapper::uniquify(func->body);
1046-
}
10471052
if (optimize && inlinedInto.size() > 0) {
10481053
OptUtils::optimizeAfterInlining(inlinedInto, module, getPassRunner());
10491054
}

‎test/lit/passes/inlining-optimizing_optimize-level=3.wast‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8418,7 +8418,7 @@
84188418
;; CHECK-NEXT: (br $__rjti$8)
84198419
;; CHECK-NEXT: )
84208420
;; CHECK-NEXT: (block $label$break$L8
8421-
;; CHECK-NEXT: (block $__rjti$28
8421+
;; CHECK-NEXT: (block $__rjti$20
84228422
;; CHECK-NEXT: (if
84238423
;; CHECK-NEXT: (i32.and
84248424
;; CHECK-NEXT: (local.tee $9
@@ -8441,15 +8441,15 @@
84418441
;; CHECK-NEXT: (local.set $8
84428442
;; CHECK-NEXT: (local.get $5)
84438443
;; CHECK-NEXT: )
8444-
;; CHECK-NEXT: (loop $while-in9
8445-
;; CHECK-NEXT: (br_if $__rjti$28
8444+
;; CHECK-NEXT: (loop $while-in5
8445+
;; CHECK-NEXT: (br_if $__rjti$20
84468446
;; CHECK-NEXT: (i32.eqz
84478447
;; CHECK-NEXT: (i32.load8_u
84488448
;; CHECK-NEXT: (local.get $8)
84498449
;; CHECK-NEXT: )
84508450
;; CHECK-NEXT: )
84518451
;; CHECK-NEXT: )
8452-
;; CHECK-NEXT: (br_if $while-in9
8452+
;; CHECK-NEXT: (br_if $while-in5
84538453
;; CHECK-NEXT: (i32.and
84548454
;; CHECK-NEXT: (local.tee $9
84558455
;; CHECK-NEXT: (i32.ne
@@ -8482,7 +8482,7 @@
84828482
;; CHECK-NEXT: (local.get $5)
84838483
;; CHECK-NEXT: )
84848484
;; CHECK-NEXT: )
8485-
;; CHECK-NEXT: (br_if $__rjti$28
8485+
;; CHECK-NEXT: (br_if $__rjti$20
84868486
;; CHECK-NEXT: (local.get $9)
84878487
;; CHECK-NEXT: )
84888488
;; CHECK-NEXT: (local.set $9
@@ -8498,15 +8498,15 @@
84988498
;; CHECK-NEXT: (local.get $8)
84998499
;; CHECK-NEXT: )
85008500
;; CHECK-NEXT: (block
8501-
;; CHECK-NEXT: (block $__rjto$010
8502-
;; CHECK-NEXT: (block $__rjti$011
8503-
;; CHECK-NEXT: (br_if $__rjti$011
8501+
;; CHECK-NEXT: (block $__rjto$06
8502+
;; CHECK-NEXT: (block $__rjti$07
8503+
;; CHECK-NEXT: (br_if $__rjti$07
85048504
;; CHECK-NEXT: (i32.le_u
85058505
;; CHECK-NEXT: (local.get $9)
85068506
;; CHECK-NEXT: (i32.const 3)
85078507
;; CHECK-NEXT: )
85088508
;; CHECK-NEXT: )
8509-
;; CHECK-NEXT: (loop $while-in312
8509+
;; CHECK-NEXT: (loop $while-in38
85108510
;; CHECK-NEXT: (if
85118511
;; CHECK-NEXT: (i32.eqz
85128512
;; CHECK-NEXT: (i32.and
@@ -8534,7 +8534,7 @@
85348534
;; CHECK-NEXT: (i32.const 4)
85358535
;; CHECK-NEXT: )
85368536
;; CHECK-NEXT: )
8537-
;; CHECK-NEXT: (br_if $while-in312
8537+
;; CHECK-NEXT: (br_if $while-in38
85388538
;; CHECK-NEXT: (i32.gt_u
85398539
;; CHECK-NEXT: (local.tee $9
85408540
;; CHECK-NEXT: (i32.sub
@@ -8545,11 +8545,11 @@
85458545
;; CHECK-NEXT: (i32.const 3)
85468546
;; CHECK-NEXT: )
85478547
;; CHECK-NEXT: )
8548-
;; CHECK-NEXT: (br $__rjti$011)
8548+
;; CHECK-NEXT: (br $__rjti$07)
85498549
;; CHECK-NEXT: )
85508550
;; CHECK-NEXT: )
85518551
;; CHECK-NEXT: )
8552-
;; CHECK-NEXT: (br $__rjto$010)
8552+
;; CHECK-NEXT: (br $__rjto$06)
85538553
;; CHECK-NEXT: )
85548554
;; CHECK-NEXT: (if
85558555
;; CHECK-NEXT: (i32.eqz
@@ -8564,7 +8564,7 @@
85648564
;; CHECK-NEXT: )
85658565
;; CHECK-NEXT: )
85668566
;; CHECK-NEXT: (local.set $9
8567-
;; CHECK-NEXT: (loop $while-in5 (result i32)
8567+
;; CHECK-NEXT: (loop $while-in59 (result i32)
85688568
;; CHECK-NEXT: (br_if $label$break$L8
85698569
;; CHECK-NEXT: (i32.eqz
85708570
;; CHECK-NEXT: (i32.load8_u
@@ -8578,7 +8578,7 @@
85788578
;; CHECK-NEXT: (i32.const 1)
85798579
;; CHECK-NEXT: )
85808580
;; CHECK-NEXT: )
8581-
;; CHECK-NEXT: (br_if $while-in5
8581+
;; CHECK-NEXT: (br_if $while-in59
85828582
;; CHECK-NEXT: (local.tee $9
85838583
;; CHECK-NEXT: (i32.sub
85848584
;; CHECK-NEXT: (local.get $9)

‎test/lit/passes/inlining-unreachable.wast‎

Lines changed: 72 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
)
3333

3434
;; CHECK: (func $call-trap-result (type $none_=>_i32) (result i32)
35-
;; CHECK-NEXT: (block $__inlined_func$trap-result (result i32)
35+
;; CHECK-NEXT: (block $__inlined_func$trap-result
3636
;; CHECK-NEXT: (unreachable)
3737
;; CHECK-NEXT: )
3838
;; CHECK-NEXT: )
@@ -77,12 +77,10 @@
7777

7878
;; CHECK: (func $caller (type $none_=>_none)
7979
;; CHECK-NEXT: (drop
80-
;; CHECK-NEXT: (block (result i32)
81-
;; CHECK-NEXT: (block $__inlined_func$callee (result i32)
82-
;; CHECK-NEXT: (br $__inlined_func$callee
83-
;; CHECK-NEXT: (call $imported
84-
;; CHECK-NEXT: (unreachable)
85-
;; CHECK-NEXT: )
80+
;; CHECK-NEXT: (block
81+
;; CHECK-NEXT: (block $__inlined_func$callee
82+
;; CHECK-NEXT: (call $imported
83+
;; CHECK-NEXT: (unreachable)
8684
;; CHECK-NEXT: )
8785
;; CHECK-NEXT: )
8886
;; CHECK-NEXT: )
@@ -102,3 +100,70 @@
102100
)
103101
)
104102
)
103+
104+
(module
105+
;; CHECK: (type $A (func))
106+
(type $A (func))
107+
108+
(func $0
109+
(nop)
110+
(call_ref $A
111+
(ref.null nofunc) ;; In Binaryen IR this makes the call_ref unreachable.
112+
)
113+
)
114+
115+
;; CHECK: (func $1 (type $A)
116+
;; CHECK-NEXT: (block $__inlined_func$0
117+
;; CHECK-NEXT: (block
118+
;; CHECK-NEXT: (nop)
119+
;; CHECK-NEXT: (unreachable)
120+
;; CHECK-NEXT: )
121+
;; CHECK-NEXT: )
122+
;; CHECK-NEXT: )
123+
(func $1 (type $A)
124+
;; After inlining, this function body will become unreachable.
125+
(call $0)
126+
)
127+
)
128+
129+
(module
130+
;; CHECK: (type $none_=>_f64 (func (result f64)))
131+
132+
;; CHECK: (func $0 (type $none_=>_f64) (result f64)
133+
;; CHECK-NEXT: (block $block
134+
;; CHECK-NEXT: (br_if $block
135+
;; CHECK-NEXT: (i32.const 0)
136+
;; CHECK-NEXT: )
137+
;; CHECK-NEXT: )
138+
;; CHECK-NEXT: (return
139+
;; CHECK-NEXT: (block
140+
;; CHECK-NEXT: (block $__inlined_func$1
141+
;; CHECK-NEXT: (block $block0
142+
;; CHECK-NEXT: (unreachable)
143+
;; CHECK-NEXT: )
144+
;; CHECK-NEXT: )
145+
;; CHECK-NEXT: )
146+
;; CHECK-NEXT: )
147+
;; CHECK-NEXT: )
148+
(func $0 (result f64)
149+
(block $block
150+
(br_if $block
151+
(i32.const 0)
152+
)
153+
)
154+
(return
155+
;; The inlined function has the same label, $block. We should not be
156+
;; confused by that when we inline the unreachable code (an error can
157+
;; occur if we mix up the two blocks or think they are identical; to avoid
158+
;; that we should fix up the duplicate labels before doing anything that
159+
;; depends on valid label names, like refinalization).
160+
(call $1)
161+
)
162+
)
163+
164+
(func $1 (result f64)
165+
(block $block
166+
(unreachable)
167+
)
168+
)
169+
)

‎test/lit/passes/inlining_enable-tail-call.wast‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@
394394
;; CHECK: (func $0
395395
;; CHECK-NEXT: (block $__inlined_func$1
396396
;; CHECK-NEXT: (call_indirect (type $T)
397-
;; CHECK-NEXT: (if (result i32)
397+
;; CHECK-NEXT: (if
398398
;; CHECK-NEXT: (i32.const 0)
399399
;; CHECK-NEXT: (unreachable)
400400
;; CHECK-NEXT: (unreachable)

‎test/lit/passes/inlining_optimize-level=3.wast‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,9 @@
149149
;; CHECK-NEXT: (drop
150150
;; CHECK-NEXT: (block (result i32)
151151
;; CHECK-NEXT: (block (result i32)
152-
;; CHECK-NEXT: (block $__inlined_func$no-calls1 (result i32)
152+
;; CHECK-NEXT: (block $__inlined_func$no-calls0 (result i32)
153153
;; CHECK-NEXT: (block (result i32)
154-
;; CHECK-NEXT: (block $__inlined_func$yes2 (result i32)
154+
;; CHECK-NEXT: (block $__inlined_func$yes1 (result i32)
155155
;; CHECK-NEXT: (i32.const 1)
156156
;; CHECK-NEXT: )
157157
;; CHECK-NEXT: )
@@ -164,7 +164,7 @@
164164
;; CHECK-NEXT: (block (result i32)
165165
;; CHECK-NEXT: (block $__inlined_func$yes-calls-but-one-use (result i32)
166166
;; CHECK-NEXT: (block (result i32)
167-
;; CHECK-NEXT: (block $__inlined_func$yes3 (result i32)
167+
;; CHECK-NEXT: (block $__inlined_func$yes2 (result i32)
168168
;; CHECK-NEXT: (i32.const 1)
169169
;; CHECK-NEXT: )
170170
;; CHECK-NEXT: )
@@ -193,7 +193,7 @@
193193
;; CHECK-NEXT: (drop
194194
;; CHECK-NEXT: (block (result i32)
195195
;; CHECK-NEXT: (block $__inlined_func$yes-loops-but-one-use (result i32)
196-
;; CHECK-NEXT: (loop $loop-in2 (result i32)
196+
;; CHECK-NEXT: (loop $loop-in0 (result i32)
197197
;; CHECK-NEXT: (i32.const 1)
198198
;; CHECK-NEXT: )
199199
;; CHECK-NEXT: )
@@ -202,7 +202,7 @@
202202
;; CHECK-NEXT: (drop
203203
;; CHECK-NEXT: (block (result i32)
204204
;; CHECK-NEXT: (block $__inlined_func$no-loops-but-one-use-but-exported (result i32)
205-
;; CHECK-NEXT: (loop $loop-in3 (result i32)
205+
;; CHECK-NEXT: (loop $loop-in2 (result i32)
206206
;; CHECK-NEXT: (i32.const 1)
207207
;; CHECK-NEXT: )
208208
;; CHECK-NEXT: )
@@ -211,7 +211,7 @@
211211
;; CHECK-NEXT: (drop
212212
;; CHECK-NEXT: (block (result i32)
213213
;; CHECK-NEXT: (block $__inlined_func$no-loops-but-one-use-but-tabled (result i32)
214-
;; CHECK-NEXT: (loop $loop-in4 (result i32)
214+
;; CHECK-NEXT: (loop $loop-in3 (result i32)
215215
;; CHECK-NEXT: (i32.const 1)
216216
;; CHECK-NEXT: )
217217
;; CHECK-NEXT: )
@@ -441,7 +441,7 @@
441441
;; CHECK: (func $foo
442442
;; CHECK-NEXT: (block $__inlined_func$bar_0
443443
;; CHECK-NEXT: (drop
444-
;; CHECK-NEXT: (block $__inlined_func$bar (result i32)
444+
;; CHECK-NEXT: (block $__inlined_func$bar
445445
;; CHECK-NEXT: (br $__inlined_func$bar_0)
446446
;; CHECK-NEXT: )
447447
;; CHECK-NEXT: )
@@ -461,8 +461,8 @@
461461
;; CHECK-NEXT: (local $0 i32)
462462
;; CHECK-NEXT: (block $__inlined_func$0_0
463463
;; CHECK-NEXT: (drop
464-
;; CHECK-NEXT: (block (result i32)
465-
;; CHECK-NEXT: (block $__inlined_func$0_0_0 (result i32)
464+
;; CHECK-NEXT: (block
465+
;; CHECK-NEXT: (block $__inlined_func$0_0_0
466466
;; CHECK-NEXT: (local.set $0
467467
;; CHECK-NEXT: (block (result i32)
468468
;; CHECK-NEXT: (br_if $__inlined_func$0_0

0 commit comments

Comments
 (0)