cfallin commented on Issue #2611:
I'll take a look -- thanks for putting this together so quickly! A few quick notes on the isel thoughts:
For example one optimization might be to not have a fuel_var and instead periodically do addq $fuel_consumed, offset(%vminterrupts_ptr) which avoids consuming extra registers. Similarly cmpq $0, offset(%vminterrupts_ptr) could be generated as well
The former is an RMW compound op and could be done pretty reasonably with the current load-op merging framework; we haven't gotten around to it yet, but it's clear how to do so if we decide it's important. The latter is actually a bit more interesting: I recently fixed a bug (#2576) that arose because of certain combinations of load-and-compare that would sink a load beyond its first use by simply disallowing compare-from-memory. That too can be re-optimized but would take more machinery.
Anyway, all that to say that there are reasons you couldn't get either of those instructions to come out the backend :-)
alexcrichton commented on Issue #2611:
Oh also to clarify, I don't think it's just isel that can be improved, but rather I think the frontend codegen could also be improved if we decide the time should be invested. For example an empty function
(func nop)
generates this x86 code with the new backend (annotated by me):0000000000000000 _wasm_function_4: ;; prologue stack-check to ensure we're not out of stack space ;; note that this is only here because function calls are made internally 0: 55 pushq %rbp 1: 48 89 e5 movq %rsp, %rbp 4: 4c 8b 17 movq (%rdi), %r10 7: 4d 8b 12 movq (%r10), %r10 a: 49 39 e2 cmpq %rsp, %r10 d: 0f 86 02 00 00 00 jbe 2 <_wasm_function_4+0x15> 13: 0f 0b ud2 ;; allocating stack space, saving registers 15: 48 83 ec 10 subq $16, %rsp 19: 4c 89 24 24 movq %r12, (%rsp) ;; load vminterrupts_ptr into r12 1d: 4c 8b 27 movq (%rdi), %r12 ;; increment the fuel consumed by 1 where 8(%r12) is where the previous count is 20: be 01 00 00 00 movl $1, %esi 25: 49 03 74 24 08 addq 8(%r12), %rsi ;; check if fuel is gerater than zero 2a: 48 31 c0 xorq %rax, %rax 2d: 48 39 c6 cmpq %rax, %rsi 30: 0f 8c 13 00 00 00 jl 19 <_wasm_function_4+0x49> ;; save the fuel count, load the intrinsic address, call it 36: 49 89 74 24 08 movq %rsi, 8(%r12) 3b: 48 8b b7 f0 06 00 00 movq 1776(%rdi), %rsi 42: ff d6 callq *%rsi ;; restore the fuel_var for this function (unnecessary instruction, happens uncondititonally after call) 44: 49 8b 74 24 08 movq 8(%r12), %rsi ;; save fuel_var at the end of the function (unnecessary instruction, happens unconditionally at function end) 49: 49 89 74 24 08 movq %rsi, 8(%r12) ;; function epilogue 4e: 4c 8b 24 24 movq (%rsp), %r12 52: 48 83 c4 10 addq $16, %rsp 56: 48 89 ec movq %rbp, %rsp 59: 5d popq %rbp 5a: c3 retq
Larger functions like https://github.com/bytecodealliance/sightglass/blob/faf101e50f7af6243fe3e910cf9455eecd9e1368/benchmarks-next/shootout-ackermann/benchmark.wat#L25-L49 end up having a lot more code generated with instrumentation enabled. I haven't yet gone through instruction-by-instruction yet, though, to see what could be improved.
github-actions[bot] commented on Issue #2611:
Subscribe to Label Action
cc @fitzgen, @peterhuene
<details>
This issue or pull request has been labeled: "cranelift", "cranelift:wasm", "fuzzing", "wasmtime:api"Thus the following users have been cc'd because of the following labels:
- fitzgen: fuzzing
- peterhuene: wasmtime:api
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
alexcrichton commented on Issue #2611:
Ok this should be rebased and updated to have
add_fuel
Last updated: Dec 23 2024 at 12:05 UTC