Stream: git-wasmtime

Topic: wasmtime / PR #8163 Cranelift: shrink ABIArgSlot

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 08:17):

jameysharp opened PR #8163 from jameysharp:shrink-abiargslot to bytecodealliance:main:

ABIArgSlot is currently 16 bytes with 4 bytes of padding. Shrinking the offset field in its Stack variant from i64 to i32 therefore reduces the enum to 8 bytes. It also reduces its alignment to 4 bytes, which might reduce padding in containing structures.

Some targets already limit stack frame sizes to 2GB or less, and in practice they must be much smaller than that, so an i32 is plenty.

By making this type only one word in size, we can put two of them in ABIArgSlotVec for free, which guarantees that such SmallVecs will never spill to the heap. In principle we could use arrayvec or something that doesn't support spilling to the heap at all.

There are many other places where we use i64 for stack frame offsets which would probably benefit from switching to i32, but I didn't want to change everything at once.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 08:17):

jameysharp requested elliottt for a review on PR #8163.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 08:17):

jameysharp requested wasmtime-compiler-reviewers for a review on PR #8163.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 09:30):

bjorn3 submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 09:30):

bjorn3 created PR review comment:

We don't currently check that the stack frame size is less than 2GB, so this unwrap can panic: https://github.com/bytecodealliance/wasmtime/issues/7916

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 16:02):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 16:02):

jameysharp created PR review comment:

That's good to know! I'll hold off on merging this until I have a chance to think more about that or somebody else fixes it.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 18 2024 at 19:01):

fitzgen submitted PR review:

LGTM, assuming we unblock it.

Last updated: Oct 23 2024 at 20:03 UTC