Skip to content

Commit d6a6188

Browse files
kripkenkripken
authored
Fix a vacuum bug with loads changing the type (#2124)
This happened on wasm2js, where implicit traps are off by default, and this bug is specific to that (less-tested) mode.
1 parent f4fd884 commit d6a6188

File tree

3 files changed

+137
-1
lines changed

3 files changed

+137
-1
lines changed

‎src/passes/Vacuum.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,12 @@ struct Vacuum : public WalkerPass<ExpressionStackWalker<Vacuum>> {
9696
// it is ok to remove a load if the result is not used, and it has no
9797
// side effects (the load itself may trap, if we are not ignoring such
9898
// things)
99+
auto* load = curr->cast<Load>();
99100
if (!resultUsed &&
100101
!EffectAnalyzer(getPassOptions(), curr).hasSideEffects()) {
101-
return curr->cast<Load>()->ptr;
102+
if (!typeMatters || load->ptr->type == type) {
103+
return load->ptr;
104+
}
102105
}
103106
return curr;
104107
}

‎test/passes/remove-unused-names_vacuum_ignore-implicit-traps.txt‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
(module
2+
(type $0 (func))
3+
(type $FUNCSIG$j (func (result i32)))
4+
(type $FUNCSIG$i (func (result i32)))
5+
(type $legaltype$wasm2js_scratch_load_i64 (func (result i32)))
6+
(type $FUNCSIG$vj (func (param i32 i32)))
7+
(type $legaltype$wasm2js_scratch_store_i64 (func (param i32 i32)))
8+
(type $FUNCSIG$ii (func (param i32) (result i32)))
9+
(type $FUNCSIG$vii (func (param i32 i32)))
10+
(type $FUNCSIG$f (func (result f32)))
11+
(type $FUNCSIG$vf (func (param f32)))
12+
(type $FUNCSIG$d (func (result f64)))
13+
(type $FUNCSIG$vd (func (param f64)))
14+
(import "env" "wasm2js_scratch_load_i32" (func $wasm2js_scratch_load_i32 (param i32) (result i32)))
15+
(import "env" "wasm2js_scratch_store_i32" (func $wasm2js_scratch_store_i32 (param i32 i32)))
16+
(import "env" "wasm2js_scratch_load_f32" (func $wasm2js_scratch_load_f32 (result f32)))
17+
(import "env" "wasm2js_scratch_store_f32" (func $wasm2js_scratch_store_f32 (param f32)))
18+
(import "env" "wasm2js_scratch_load_f64" (func $wasm2js_scratch_load_f64 (result f64)))
19+
(import "env" "wasm2js_scratch_store_f64" (func $wasm2js_scratch_store_f64 (param f64)))
20+
(import "env" "getTempRet0" (func $getTempRet0 (result i32)))
21+
(import "env" "wasm2js_scratch_load_i64" (func $legalimport$wasm2js_scratch_load_i64 (result i32)))
22+
(import "env" "wasm2js_scratch_store_i64" (func $legalimport$wasm2js_scratch_store_i64 (param i32 i32)))
23+
(import "env" "wasm2js_scratch_load_i64" (func $wasm2js_scratch_load_i64 (result i32)))
24+
(import "env" "wasm2js_scratch_store_i64" (func $wasm2js_scratch_store_i64 (param i32 i32)))
25+
(memory $0 1 1)
26+
(global $i64toi32_i32$HIGH_BITS (mut i32) (i32.const 0))
27+
(func $0 (; 11 ;) (type $0)
28+
(local $0 f64)
29+
(local $1 i32)
30+
(nop)
31+
)
32+
(func $legalfunc$wasm2js_scratch_load_i64 (; 12 ;) (type $FUNCSIG$j) (result i32)
33+
(local $0 i32)
34+
(local $1 i32)
35+
(local.set $1
36+
(i32.or
37+
(call $legalimport$wasm2js_scratch_load_i64)
38+
(i32.const 0)
39+
)
40+
)
41+
(global.set $i64toi32_i32$HIGH_BITS
42+
(call $getTempRet0)
43+
)
44+
(local.get $1)
45+
)
46+
(func $legalfunc$wasm2js_scratch_store_i64 (; 13 ;) (type $FUNCSIG$vj) (param $0 i32) (param $1 i32)
47+
(call $legalimport$wasm2js_scratch_store_i64
48+
(local.get $0)
49+
(local.get $1)
50+
)
51+
)
52+
)

‎test/passes/remove-unused-names_vacuum_ignore-implicit-traps.wast‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
(module
2+
(type $0 (func))
3+
(type $FUNCSIG$j (func (result i32)))
4+
(type $FUNCSIG$i (func (result i32)))
5+
(type $legaltype$wasm2js_scratch_load_i64 (func (result i32)))
6+
(type $FUNCSIG$vj (func (param i32 i32)))
7+
(type $legaltype$wasm2js_scratch_store_i64 (func (param i32 i32)))
8+
(import "env" "wasm2js_scratch_load_i32" (func $wasm2js_scratch_load_i32 (param i32) (result i32)))
9+
(import "env" "wasm2js_scratch_store_i32" (func $wasm2js_scratch_store_i32 (param i32 i32)))
10+
(import "env" "wasm2js_scratch_load_f32" (func $wasm2js_scratch_load_f32 (result f32)))
11+
(import "env" "wasm2js_scratch_store_f32" (func $wasm2js_scratch_store_f32 (param f32)))
12+
(import "env" "wasm2js_scratch_load_f64" (func $wasm2js_scratch_load_f64 (result f64)))
13+
(import "env" "wasm2js_scratch_store_f64" (func $wasm2js_scratch_store_f64 (param f64)))
14+
(import "env" "getTempRet0" (func $getTempRet0 (result i32)))
15+
(import "env" "wasm2js_scratch_load_i64" (func $legalimport$wasm2js_scratch_load_i64 (result i32)))
16+
(import "env" "wasm2js_scratch_store_i64" (func $legalimport$wasm2js_scratch_store_i64 (param i32 i32)))
17+
(import "env" "wasm2js_scratch_load_i64" (func $wasm2js_scratch_load_i64 (result i64)))
18+
(import "env" "wasm2js_scratch_store_i64" (func $wasm2js_scratch_store_i64 (param i64)))
19+
(memory $0 1 1)
20+
(global $i64toi32_i32$HIGH_BITS (mut i32) (i32.const 0))
21+
(func $0 (; 11 ;) (type $0)
22+
(local $0 f64)
23+
(local $1 i32)
24+
(block $label$1
25+
(block
26+
(nop)
27+
(drop
28+
(if (result f64)
29+
(i32.const 1)
30+
(block (result f64)
31+
(nop)
32+
(f64.load
33+
(i32.const 19)
34+
)
35+
)
36+
(block
37+
(drop
38+
(f64.const 1)
39+
)
40+
(br $label$1)
41+
(nop)
42+
)
43+
)
44+
)
45+
)
46+
(nop)
47+
)
48+
)
49+
(func $legalfunc$wasm2js_scratch_load_i64 (; 12 ;) (result i32)
50+
(local $0 i32)
51+
(local $1 i32)
52+
(local.set $1
53+
(i32.or
54+
(call $legalimport$wasm2js_scratch_load_i64)
55+
(block (result i32)
56+
(nop)
57+
(block (result i32)
58+
(nop)
59+
(i32.const 0)
60+
)
61+
)
62+
)
63+
)
64+
(global.set $i64toi32_i32$HIGH_BITS
65+
(call $getTempRet0)
66+
)
67+
(local.get $1)
68+
)
69+
(func $legalfunc$wasm2js_scratch_store_i64 (; 13 ;) (param $0 i32) (param $1 i32)
70+
(call $legalimport$wasm2js_scratch_store_i64
71+
(block (result i32)
72+
(nop)
73+
(local.get $0)
74+
)
75+
(block (result i32)
76+
(nop)
77+
(local.get $1)
78+
)
79+
)
80+
)
81+
)

0 commit comments

Comments
 (0)