Skip to content

Commit f5b8221

Browse files
kripkenkripken
committed
Improve selectification in remove-unused-brs
We turned an if into a select when optimizing for size (and if side effects etc. allow so). This patch improves that, doing it not just when optimizing for size, but also when it looks beneficial given the amount of work on both sides of the if. As a result we can create selects in -O3 etc.
1 parent e6048c1 commit f5b8221

File tree

6 files changed

+126
-71
lines changed

6 files changed

+126
-71
lines changed

‎src/passes/RemoveUnusedBrs.cpp‎

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121
#include <wasm.h>
2222
#include <pass.h>
2323
#include <parsing.h>
24-
#include <ir/utils.h>
2524
#include <ir/branch-utils.h>
25+
#include <ir/cost.h>
2626
#include <ir/effects.h>
27+
#include <ir/utils.h>
2728
#include <wasm-builder.h>
2829

2930
namespace wasm {
@@ -780,27 +781,44 @@ struct RemoveUnusedBrs : public WalkerPass<PostWalker<RemoveUnusedBrs>> {
780781

781782
void visitIf(If* curr) {
782783
// we may have simplified ifs enough to turn them into selects
783-
// this is helpful for code size, but can be a tradeoff with performance as we run both code paths
784-
if (!shrink) return;
785-
if (curr->ifFalse && isConcreteType(curr->ifTrue->type) && isConcreteType(curr->ifFalse->type)) {
786-
// if with else, consider turning it into a select if there is no control flow
787-
// TODO: estimate cost
788-
EffectAnalyzer condition(passOptions, curr->condition);
789-
if (!condition.hasSideEffects()) {
790-
EffectAnalyzer ifTrue(passOptions, curr->ifTrue);
791-
if (!ifTrue.hasSideEffects()) {
792-
EffectAnalyzer ifFalse(passOptions, curr->ifFalse);
793-
if (!ifFalse.hasSideEffects()) {
794-
auto* select = getModule()->allocator.alloc<Select>();
795-
select->condition = curr->condition;
796-
select->ifTrue = curr->ifTrue;
797-
select->ifFalse = curr->ifFalse;
798-
select->finalize();
799-
replaceCurrent(select);
800-
}
784+
if (auto* select = selectify(curr)) {
785+
replaceCurrent(select);
786+
}
787+
}
788+
789+
// Convert an if into a select, if possible and beneficial to do so.
790+
Select* selectify(If* iff) {
791+
if (!iff->ifFalse ||
792+
!isConcreteType(iff->ifTrue->type) ||
793+
!isConcreteType(iff->ifFalse->type)) {
794+
return nullptr;
795+
}
796+
// This is always helpful for code size, but can be a tradeoff with performance
797+
// as we run both code paths. So when shrinking we always try to do this, but
798+
// otherwise must consider more carefully.
799+
if (!passOptions.shrinkLevel) {
800+
// Consider the cost of executing all the code unconditionally
801+
const auto MAX_COST = 7;
802+
auto total = CostAnalyzer(iff->ifTrue).cost +
803+
CostAnalyzer(iff->ifFalse).cost;
804+
if (total >= MAX_COST) return nullptr;
805+
}
806+
// Check if side effects allow this.
807+
EffectAnalyzer condition(passOptions, iff->condition);
808+
if (!condition.hasSideEffects()) {
809+
EffectAnalyzer ifTrue(passOptions, iff->ifTrue);
810+
if (!ifTrue.hasSideEffects()) {
811+
EffectAnalyzer ifFalse(passOptions, iff->ifFalse);
812+
if (!ifFalse.hasSideEffects()) {
813+
return Builder(*getModule()).makeSelect(
814+
iff->condition,
815+
iff->ifTrue,
816+
iff->ifFalse
817+
);
801818
}
802819
}
803820
}
821+
return nullptr;
804822
}
805823

806824
void visitSetLocal(SetLocal* curr) {

‎test/binaryen.js/optimize-levels.js.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ shrinkLevel=0
5252
(type $i (func (param i32) (result i32)))
5353
(export "test" (func $test))
5454
(func $test (; 0 ;) (type $i) (param $0 i32) (result i32)
55-
(if (result i32)
56-
(get_local $0)
55+
(select
5756
(get_local $0)
5857
(i32.const 0)
58+
(get_local $0)
5959
)
6060
)
6161
)

‎test/passes/1.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,15 @@
1919
(func $ifs (; 4 ;) (type $2) (param $0 i32) (result i32)
2020
(if (result i32)
2121
(get_local $0)
22-
(if (result i32)
23-
(get_local $0)
22+
(select
2423
(i32.const 2)
2524
(i32.const 3)
26-
)
27-
(if (result i32)
2825
(get_local $0)
26+
)
27+
(select
2928
(i32.const 4)
3029
(i32.const 5)
30+
(get_local $0)
3131
)
3232
)
3333
)

‎test/passes/O.txt‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
(module
22
(type $0 (func (result i32)))
33
(type $1 (func (param i64)))
4+
(type $2 (func (param i32) (result i32)))
45
(export "ret" (func $ret))
56
(export "waka" (func $if-0-unreachable-to-none))
7+
(export "many-selects" (func $many-selects))
68
(func $ret (; 0 ;) (; has Stack IR ;) (type $0) (result i32)
79
(drop
810
(call $ret)
@@ -18,4 +20,23 @@
1820
(func $if-0-unreachable-to-none (; 1 ;) (; has Stack IR ;) (type $1) (param $0 i64)
1921
(unreachable)
2022
)
23+
(func $many-selects (; 2 ;) (; has Stack IR ;) (type $2) (param $0 i32) (result i32)
24+
(tee_local $0
25+
(select
26+
(i32.const -1073741824)
27+
(select
28+
(i32.const 1073741823)
29+
(get_local $0)
30+
(i32.gt_s
31+
(get_local $0)
32+
(i32.const 1073741823)
33+
)
34+
)
35+
(i32.lt_s
36+
(get_local $0)
37+
(i32.const -1073741824)
38+
)
39+
)
40+
)
41+
)
2142
)

‎test/passes/O.wast‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,26 @@
2424
)
2525
)
2626
)
27+
(func $many-selects (export "many-selects") (param $0 i32) (result i32)
28+
(if
29+
(i32.lt_s
30+
(get_local $0)
31+
(i32.const -1073741824)
32+
)
33+
(set_local $0
34+
(i32.const -1073741824)
35+
)
36+
(if
37+
(i32.gt_s
38+
(get_local $0)
39+
(i32.const 1073741823)
40+
)
41+
(set_local $0
42+
(i32.const 1073741823)
43+
)
44+
)
45+
)
46+
(get_local $0)
47+
)
2748
)
2849

‎test/passes/remove-unused-brs.txt‎

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,7 @@
179179
)
180180
)
181181
(func $b14 (; 14 ;) (type $2) (result i32)
182-
(if (result i32)
183-
(i32.const 1)
182+
(select
184183
(block $topmost (result i32)
185184
(block $block1 (result i32)
186185
(i32.const 12)
@@ -189,6 +188,7 @@
189188
(block $block3 (result i32)
190189
(i32.const 27)
191190
)
191+
(i32.const 1)
192192
)
193193
)
194194
(func $b15 (; 15 ;) (type $1)
@@ -726,10 +726,10 @@
726726
(i32.const 6)
727727
)
728728
)
729-
(if (result i32)
730-
(i32.const 6)
729+
(select
731730
(i32.const 7)
732731
(i32.const 8)
732+
(i32.const 6)
733733
)
734734
)
735735
)
@@ -1861,10 +1861,10 @@
18611861
(i32.const 0)
18621862
)
18631863
(func $if-flow-1 (; 75 ;) (type $2) (result i32)
1864-
(if (result i32)
1865-
(i32.const 0)
1864+
(select
18661865
(i32.const 1)
18671866
(i32.const 2)
1867+
(i32.const 0)
18681868
)
18691869
)
18701870
(func $if-flow-2 (; 76 ;) (type $2) (result i32)
@@ -2073,12 +2073,12 @@
20732073
)
20742074
)
20752075
(func $if-block-br-5-value (; 93 ;) (type $2) (result i32)
2076-
(if (result i32)
2077-
(i32.const 1)
2076+
(select
20782077
(block $label (result i32)
20792078
(i32.const 2)
20802079
)
20812080
(i32.const 3)
2081+
(i32.const 1)
20822082
)
20832083
)
20842084
(func $restructure-if-outerType-change (; 94 ;) (type $1)
@@ -2249,11 +2249,7 @@
22492249
)
22502250
)
22512251
(drop
2252-
(if (result i32)
2253-
(i32.eq
2254-
(get_local $x)
2255-
(i32.const 1)
2256-
)
2252+
(select
22572253
(i32.add
22582254
(i32.const 2)
22592255
(i32.const 3)
@@ -2262,27 +2258,31 @@
22622258
(i32.const 2)
22632259
(i32.const 3)
22642260
)
2261+
(i32.eq
2262+
(get_local $x)
2263+
(i32.const 1)
2264+
)
22652265
)
22662266
)
22672267
)
22682268
(func $if-one-side (; 103 ;) (type $2) (result i32)
22692269
(local $x i32)
2270-
(if
2271-
(i32.const 1)
2272-
(set_local $x
2270+
(set_local $x
2271+
(select
22732272
(i32.const 2)
2273+
(get_local $x)
2274+
(i32.const 1)
22742275
)
22752276
)
22762277
(get_local $x)
22772278
)
22782279
(func $if-one-side-b (; 104 ;) (type $2) (result i32)
22792280
(local $x i32)
2280-
(if
2281-
(i32.eqz
2282-
(i32.const 1)
2283-
)
2284-
(set_local $x
2281+
(set_local $x
2282+
(select
2283+
(get_local $x)
22852284
(i32.const 2)
2285+
(i32.const 1)
22862286
)
22872287
)
22882288
(get_local $x)
@@ -2297,14 +2297,12 @@
22972297
(local $z i32)
22982298
(drop
22992299
(call $if-one-side-tee-etc
2300-
(block (result i32)
2301-
(if
2300+
(tee_local $x
2301+
(select
2302+
(i32.const -4)
2303+
(get_local $x)
23022304
(i32.const -3)
2303-
(set_local $x
2304-
(i32.const -4)
2305-
)
23062305
)
2307-
(get_local $x)
23082306
)
23092307
)
23102308
)
@@ -2313,13 +2311,15 @@
23132311
(func $ifs-copies-recursive (; 106 ;) (type $10) (param $20 i32) (result i32)
23142312
(if
23152313
(i32.const 1)
2316-
(if
2317-
(i32.const 2)
2318-
(if
2319-
(i32.const 3)
2320-
(set_local $20
2314+
(set_local $20
2315+
(select
2316+
(select
23212317
(i32.const 4)
2318+
(get_local $20)
2319+
(i32.const 3)
23222320
)
2321+
(get_local $20)
2322+
(i32.const 2)
23232323
)
23242324
)
23252325
)
@@ -2329,12 +2329,11 @@
23292329
(local $x i32)
23302330
(local $y i32)
23312331
(loop $top
2332-
(if
2333-
(i32.eqz
2334-
(i32.const 1)
2335-
)
2336-
(set_local $x
2332+
(set_local $x
2333+
(select
2334+
(get_local $x)
23372335
(get_local $y)
2336+
(i32.const 1)
23382337
)
23392338
)
23402339
(br $top)
@@ -2372,16 +2371,12 @@
23722371
(local $y i32)
23732372
(loop $top
23742373
(drop
2375-
(block (result i32)
2376-
(if
2377-
(i32.eqz
2378-
(i32.const 1)
2379-
)
2380-
(set_local $x
2381-
(i32.const 2)
2382-
)
2374+
(tee_local $x
2375+
(select
2376+
(get_local $x)
2377+
(i32.const 2)
2378+
(i32.const 1)
23832379
)
2384-
(get_local $x)
23852380
)
23862381
)
23872382
(br $top)

0 commit comments

Comments
 (0)