Wasmtime GitHub notifications bot (Aug 30 2024 at 15:28): Wasmtime GitHub notifications bot (Aug 30 2024 at 15:28):alexcrichton requested fitzgen for a review on PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 15:28):alexcrichton requested wasmtime-compiler-reviewers for a review on PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 15:28):alexcrichton opened PR #9190 from alexcrichton:refactor-call-info to bytecodealliance:main:
This commit is aimed at making it easier to extend
CallInfoamongst the backends with more fields. Most backends already share almost all the same fields except for minor differences so this commit moves backends to all roughly the same track for handling this to enable having one definition ofCallInfothat is shared everywhere.
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:10):fitzgen submitted PR review.
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:10):fitzgen created PR review comment:
I don't think this is anything we necessarily need to address right now, and some backends are already like this, but by not having the name in the
CallInfowe are inflating the size of the instruction a bit. The downside is that now we would need direct vs indirect variants, if we kept everything inside the boxedCallInfo.
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:10):fitzgen created PR review comment:
This seems unrelated?
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:10):fitzgen created PR review comment:
Nevermind, I see that this is factored out from the call implementation
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:16):bjorn3 submitted PR review.
Wasmtime GitHub notifications bot (Aug 30 2024 at 16:16):bjorn3 created PR review comment:
Maybe this should accept CallInfo pre-boxed? That would save some copying around.
Wasmtime GitHub notifications bot (Aug 30 2024 at 17:46):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 17:48):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 17:49):alexcrichton submitted PR review.
Wasmtime GitHub notifications bot (Aug 30 2024 at 17:49):alexcrichton created PR review comment:
It's true yeah, I was assuming that the x64 style of moving indirect-vs-not into the
Instwas ok to apply to all backends. If we end up undoing that though backends could also have something likestruct X64CallInfo { info: CallInfo, ... }or similar
Wasmtime GitHub notifications bot (Aug 30 2024 at 17:49):alexcrichton has enabled auto merge for PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 22:45):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 22:45):alexcrichton has disabled auto merge for PR #9190.
Wasmtime GitHub notifications bot (Aug 30 2024 at 22:45):alexcrichton commented on PR #9190:
Ok, as predicted size assertions got the best of me. I've refactored the other direction for backends to include the name/destination in the
CallInfouniformly across all backends.That was a large-ish change though so mind taking another look @fitzgen?
Wasmtime GitHub notifications bot (Aug 30 2024 at 23:09):fitzgen commented on PR #9190:
I don't have time today, and I'm going backpacking next week, so while I don't mind taking a look when I get back you might want to find another reviewer :-p
Wasmtime GitHub notifications bot (Aug 30 2024 at 23:25):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Sep 03 2024 at 14:24):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Sep 03 2024 at 14:24):alexcrichton commented on PR #9190:
@elliottt would you be ok reviewing this?
Wasmtime GitHub notifications bot (Sep 03 2024 at 18:06):elliottt submitted PR review:
This all looks good to me!
Wasmtime GitHub notifications bot (Sep 03 2024 at 18:39):alexcrichton updated PR #9190.
Wasmtime GitHub notifications bot (Sep 03 2024 at 18:39):alexcrichton has enabled auto merge for PR #9190.
Wasmtime GitHub notifications bot (Sep 03 2024 at 19:13):alexcrichton merged PR #9190.
Last updated: Oct 23 2024 at 20:03 UTC