Stream: git-wasmtime

Topic: wasmtime / PR #8664 cranelift: Always consider sret argum...

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:51):

jameysharp opened PR #8664 from jameysharp:fix-8659 to bytecodealliance:main:

In #8438 we stopped emitting register bindings for unused arguments, based on the use-counts from compute_use_states. However, that doesn't count the use of the struct-return argument that's automatically added after lowering when the rets instruction is generated in the epilogue. As a result, using a struct-return argument caused register allocation to panic due to the VReg not being defined anywhere.

This commit adds a use to the struct-return argument so that it's always available in the epilogue.

Fixes #8659

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:51):

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

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:51):

jameysharp requested fitzgen for a review on PR #8664.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:54):

cfallin submitted PR review:

LGTM!

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:54):

cfallin submitted PR review:

LGTM!

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 17:54):

cfallin created PR review comment:

Could we add a comment here to note it starts at Once (for the one use that is the implicit return), but can still be updated below?

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 18:04):

jameysharp submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 18:04):

jameysharp created PR review comment:

Actually, should I just start it at Multiple in case there are multiple epilogues? The definition of the sret vreg can't participate in load-sinking or anything so I don't think the value really matters, but I guess it should be "correct" anyway?

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 18:18):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 18:18):

cfallin created PR review comment:

Oh, yes, that makes more sense actually, I agree. It's unlikely to matter as you say but might as well get the analysis right...

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 19:04):

jameysharp updated PR #8664.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 19:05):

jameysharp has enabled auto merge for PR #8664.

view this post on Zulip Wasmtime GitHub notifications bot (May 20 2024 at 19:27):

jameysharp merged PR #8664.

Last updated: Nov 22 2024 at 17:03 UTC