bytecodealliance / bytecodealliance/wasmtime

Useless stack frame allocation in Cranelift

Open
#4,473 8 comments 0 reactions 0 assignees View on GitHub
cranelift cranelift:area:regalloc cranelift:area:x64
Dominant language
Rust
Stars
18.6k
Forks
1.8k
Avg merge
1d 18h
Merged PRs (30d)
126

Description

Researching the stack frame allocation logic in Cranelift, I've came across a behavior I consider to be suboptimal, which I cannot explain.

I generated void N-ary WASM functions with empty body for N=[1..99]. Arguments are `i64` for the matter of simplicity. Then I compiled them all for `x86_64-none-linux-gnu` target and explored the machine code generated.

Functions with arity from 1 to 4 shows some minimal yet useless preamble/postamble code generated:

```asm
push %rbp
mov %rsp,%rbp
mov %rbp,%rsp
pop %rbp
ret
```

Starting from arity 5, argument loading code is generated, although values loaded are never used (the example is 7-ary func):

```asm
push %rbp
mov %rsp,%rbp
mov 0x10(%rbp),%rax
mov 0x18(%rbp),%r10
mov 0x20(%rbp),%r11
mov %rbp,%rsp
pop %rbp
ret
```

Starting from arity 8, stack frame is generated as a result of `num_spillslots` from regalloc2 growing lineary with the number of arguments:

```asm
<_wasm_function_0>:
push %rbp
mov %rsp,%rbp
mov 0x8(%rdi),%r10
mov (%r10),%r10
add $0x10,%r10
cmp %rsp,%r10
jbe <_wasm_function_0+0x1a>
ud2
sub $0x20,%rsp
mov %r15,0x10(%rsp)
mov 0x10(%rbp),%rax
mov 0x18(%rbp),%r10
mov 0x20(%rbp),%r11
mov 0x28(%rbp),%r15
mov 0x10(%rsp),%r15
add $0x20,%rsp
mov %rbp,%rsp
pop %rbp
ret
```

The higher the arity, the higher the frame size. For 99-ary function, 784-byte frame is generated, although it obviously cannot be used for anything by an empty-body function, which looks like a problem to me.

Besides that, higher arities produce really weird argument loading code which just load values to registers only to overwrite them with other values at once:

```asm
...
mov 0x60(%rbp),%rsi
mov 0x68(%rbp),%rsi
mov 0x70(%rbp),%rsi
mov 0x78(%rbp),%rcx
mov 0x80(%rbp),%rcx
mov 0x88(%rbp),%rcx
...
```

Tested with the tip of `master` branch of `wasmtime`, as of today.

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.