bytecodealliance / bytecodealliance/wasmtime

Unnecessary stack checks and prologue for wasm functions containing a single `return_call_ref`

Open
#14,168 0 comments 0 reactions 0 assignees View on GitHub
cranelift:goal:optimize-speed
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 18h
Merged PRs (30d)
126

Description

Given this input:

```wasm
(module
(type $t (func))
(global (export "g") (mut (ref $t)) (ref.func $f))
(func $f)

(func (export "call")
(return_call_ref $t (global.get 0))
)
)
```

this currently compiles as:

```
$ wasmtime compile foo.wat
$ wasmtime objdump foo.cwasm --filter 1
wasm[0]::function[1]:
pushq %rbp
movq %rsp, %rbp
movq 8(%rdi), %r10
movq 0x18(%r10), %r10
addq $0x10, %r10
cmpq %rsp, %r10
ja 0x52
movq 0x30(%rdi), %rsi
movq %rdi, %r8
movq 8(%rsi), %r10
╰─╼ trap: Normal(NullReference)
movq 0x18(%rsi), %rdi
movq %r8, %rsi
movq %rbp, %rsp
popq %rbp
jmpq *%r10
ud2
╰─╼ trap: Normal(StackOverflow)
```

There's a few inefficiencies with this codegen we could improve upon:

* There's an explicit stack check as `cmpq %rsp, %r10` here which isn't necessary. That's injected for functions in general which call another function, but it's not injected for leaf functions, and in this case this is sort of a leaf function.
* This sets up a prologue/epilogue which isn't necessary for this "mostly leaf" function.
* There's a trap registered for a null function reference, but this function reference type can't be null so that's not necessary.

Nothing major here, but might be nice to clean up some of this as well.

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.