bytecodealliance / bytecodealliance/wasmtime

Cranelift: don't hoist instructions whose results are only used in cold blocks

Open
#8,197 1 comment 0 reactions 0 assignees View on GitHub
bug cranelift
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 18h
Merged PRs (30d)
126

Description

### `.clif` Test Case

Wasmtime currently translates this WebAssembly module:

```lisp
(module
(type $fn (func (result i32)))
(table $fnptrs 2 2 funcref)
(func (param i32)
loop
local.get 0
call_indirect $fnptrs (type $fn)
br 0
end))
```

To this CLIF, before optimization:

```
function u0:0(i64 vmctx, i64, i32) fast {
gv0 = vmctx
gv1 = load.i64 notrap aligned readonly gv0+8
gv2 = load.i64 notrap aligned gv1
gv3 = vmctx
gv4 = load.i64 notrap aligned gv3+72
sig0 = (i64 vmctx, i64) -> i32 fast
sig1 = (i64 vmctx, i32 uext, i32 uext) -> i64 system_v
stack_limit = gv2

block0(v0: i64, v1: i64, v2: i32):
v3 -> v2
v28 -> v3
@0027 jump block2

block2:
@002b v4 = iconst.i32 2
@002b v5 = icmp.i32 uge v3, v4 ; v4 = 2
@002b v6 = uextend.i64 v3
@002b v7 = global_value.i64 gv4
@002b v8 = ishl_imm v6, 3
@002b v9 = iadd v7, v8
@002b v10 = iconst.i64 0
@002b v11 = select_spectre_guard v5, v10, v9 ; v10 = 0
@002b v12 = load.i64 table_oob aligned table v11
@002b v13 = band_imm v12, -2
@002b brif v12, block5(v13), block4

block4 cold:
@002b v15 = iconst.i32 0
@002b v16 = global_value.i64 gv3
@002b v17 = load.i64 notrap aligned readonly v16+56
@002b v18 = load.i64 notrap aligned readonly v17+72
@002b v19 = call_indirect sig1, v18(v16, v15, v3) ; v15 = 0
@002b jump block5(v19)

block5(v14: i64):
@002b v20 = global_value.i64 gv3
@002b v21 = load.i64 notrap aligned readonly v20+64
@002b v22 = load.i32 notrap aligned readonly v21
@002b v23 = load.i32 icall_null aligned readonly v14+24
@002b v24 = icmp eq v23, v22
@002b trapz v24, bad_sig
@002b v25 = load.i64 notrap aligned readonly v14+16
@002b v26 = load.i64 notrap aligned readonly v14+32
@002b v27 = call_indirect sig0, v25(v26, v0)
@002e jump block2
}
```

### Steps to Reproduce

Optimize this CLIF with the egraph pass enabled using `opt_level=speed`.

### Expected Results

I think the loads of `v17` and `v18` should stay in the `cold` block.

### Actual Results

The loads of `v17` and `v18` are hoisted to the entry block, since they have the `notrap` and `readonly` flags set and depend only on the vmctx parameter, `v0`.

### Versions and Environment

Cranelift version or commit: tested on afaf1c73f67c42e638590608579b2b02c7f65ca3

### Extra Info

This is not a correctness bug but might be a performance problem. I haven't measured it, just noticed it while looking at the CLIF that Wasmtime currently generates for `call_indirect`.

cc: @cfallin

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.