Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34): Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34):MarinPostma opened PR #9990 from MarinPostma:winch-x64-rmw-add to bytecodealliance:main:
impl read-modify-write add for winch:
i32.atomic.rmw8.add_ui32.atomic.rmw16.add_ui32.atomic.rmw.addi64.atomic.rmw8.add_ui64.atomic.rmw16.add_ui64.atomic.rmw32.add_ui64.atomic.rmw.add
Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34):MarinPostma requested cfallin for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34):MarinPostma requested wasmtime-compiler-reviewers for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34):MarinPostma requested wasmtime-core-reviewers for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 13 2025 at 00:34):MarinPostma requested dicej for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 13 2025 at 00:37):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 13 2025 at 02:12):github-actions[bot] commented on PR #9990:
Subscribe to Label Action
cc @saulecabrera
<details>
This issue or pull request has been labeled: "winch"Thus the following users have been cc'd because of the following labels:
- saulecabrera: winch
To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.
Learn more.
</details>
Wasmtime GitHub notifications bot (Jan 13 2025 at 14:16):saulecabrera commented on PR #9990:
I can take this review.
Wasmtime GitHub notifications bot (Jan 13 2025 at 14:16):saulecabrera requested saulecabrera for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 11:33):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 11:40):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 11:44):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 15:56):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 14 2025 at 15:56):saulecabrera created PR review comment:
The extension kinds used in this case are signed, however the spec states zero extension instead. I believe we need to augment
ExtendKindto account for the missing unsigned extensions.
Wasmtime GitHub notifications bot (Jan 14 2025 at 15:56):saulecabrera created PR review comment:
pub(crate) fn emit_atomic_rmw(
Wasmtime GitHub notifications bot (Jan 14 2025 at 16:10):MarinPostma submitted PR review.
Wasmtime GitHub notifications bot (Jan 14 2025 at 16:10):MarinPostma created PR review comment:
that's because https://github.com/bytecodealliance/wasmtime/blob/main/winch/codegen/src/isa/x64/asm.rs#L395-L401 takes
ExtendKind, even thoughmovzxzero-extend. Can I do that in a followup, if thats ok? i'll add variants for unsigned conversions, and update any affected code.
Wasmtime GitHub notifications bot (Jan 14 2025 at 16:11):MarinPostma edited PR review comment.
Wasmtime GitHub notifications bot (Jan 14 2025 at 17:58):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 17:59):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 14 2025 at 20:56):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 14 2025 at 20:56):saulecabrera created PR review comment:
I've extended
ExtendKindhere https://github.com/bytecodealliance/wasmtime/pull/10012 to include unsigned extends.
Wasmtime GitHub notifications bot (Jan 15 2025 at 12:04):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 15 2025 at 12:04):saulecabrera created PR review comment:
Just a note that this will require a rebase once https://github.com/bytecodealliance/wasmtime/pull/9987 lands and use
emit_compute_heap_address_align_checked
Wasmtime GitHub notifications bot (Jan 15 2025 at 13:06):MarinPostma commented on PR #9990:
@saulecabrera I'll do all the rebases once #9987 is in
Wasmtime GitHub notifications bot (Jan 15 2025 at 14:35):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 14:42):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 14:42):MarinPostma requested saulecabrera for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 14:44):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 14:44):MarinPostma commented on PR #9990:
@saulecabrera rebase is done. I have also added a check to ensure that only valid extends are passed to
emit_atomic_rmw
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:01):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:01):saulecabrera created PR review comment:
I believe the check here needs updating? The code is emitting a zero extend but matching against a signed extend? Also I believe that to: (i) reduce duplication and (ii) avoid affecting compile time performance, we can entirely remove this check, since Cranelift's emission layer already takes care of it https://github.com/bytecodealliance/wasmtime/blob/main/cranelift/codegen/src/isa/x64/inst/emit.rs#L973.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:04):MarinPostma submitted PR review.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:04):MarinPostma created PR review comment:
good catch i'll remove it
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:06):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:07):MarinPostma requested saulecabrera for a review on PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:23):MarinPostma updated PR #9990.
Wasmtime GitHub notifications bot (Jan 15 2025 at 15:41):saulecabrera submitted PR review.
Wasmtime GitHub notifications bot (Jan 15 2025 at 16:01):saulecabrera merged PR #9990.
Last updated: Jan 24 2025 at 00:11 UTC