bytecodealliance / wasmtime

A fast and secure runtime for WebAssembly
https://wasmtime.dev/
Apache License 2.0
14.81k stars 1.24k forks source link

riscv64: Update `Inst::worst_case_size` #8850

Closed afonso360 closed 1 week ago

afonso360 commented 1 week ago

👋 Hey,

This PR updates the Inst::worst_case_size size for the RISC-V backend. The panic that happens in #8847 is entirely due to the return_call_indirect generating too many bytes.

I found it difficult to add an automatic calculation of the worst possible size for that instruction to the test that we have, so I attempted to manually calculate the worst case size and used that.

The two test cases here are the original test case, and a minimized version without zicond. I'm not entirely sure why it bisects to the ZiCond PR (https://github.com/bytecodealliance/wasmtime/pull/8695), but having both test cases is not too much of a burden, so might as well.

The increased worst case size now causes an island to be emitted in the return-call.clif test, which are the changes for that test in this PR.

alexcrichton commented 1 week ago

I'm not entirely sure why it bisects to the ZiCond PR (https://github.com/bytecodealliance/wasmtime/pull/8695)

In retrospect this is my fault. The test case in #8847 uses has_zicond so during bisection I marked "unknown feature has_zicond" as "good" which pointed to the ZiCond PR. I suspect if I had just removed the zicond feature itself I would have found a different point of bisection.

Sorry about that, should have dug in a bit further myself! I naively assumed that the bug was in the zicond instructions added but in retrospect I see how that doesn't make sense