Stream: git-wasmtime

Topic: wasmtime / PR #10998 winch: Use `Masm::with_scratch` in I...

Wasmtime GitHub notifications bot (Jun 10 2025 at 15:21):

saulecabrera opened PR #10998 from saulecabrera:winch-use-masm-with-scratch to bytecodealliance:main:

This commit is a follow-up to
https://github.com/bytecodealliance/wasmtime/pull/10989; it migrates all the uses of the scratch register to use Masm::with_scratch. This commit also updates the tests that can fail / are ignored, since many of the bugs were introduced by accidental clobbers to scratch registers.

Even though this change doesn't introduce any functional changes, the disassembly changes in aarch64 are due to the usage of x17 as an extra scratch registers. The updates in x64 dissembly are related to offset changes.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jun 10 2025 at 15:24):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 15:24):

saulecabrera created PR review comment:

Oh this is wrong, it's using the wrong scratch type. I'll take a look.

Wasmtime GitHub notifications bot (Jun 10 2025 at 15:59):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 15:59):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:00):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:00):

saulecabrera created PR review comment:

Fixed in https://github.com/bytecodealliance/wasmtime/pull/10998/commits/f6529682aae214d898e303ccbd46a4794ef30415

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:01):

saulecabrera edited PR #10998:

This commit is a follow-up to
https://github.com/bytecodealliance/wasmtime/pull/10989; it migrates all the uses of the scratch register to use Masm::with_scratch. This commit also updates the tests that can fail / are ignored, since many of the bugs were introduced by accidental clobbers to scratch registers.

Even though this change doesn't introduce any functional changes, the disassembly changes in aarch64 are due to the usage of x17 as an extra scratch registers.
<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:07):

alexcrichton submitted PR review:

Very nice :+1:

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:07):

alexcrichton created PR review comment:

WDYT about something like:

trait MacroAssembler {
    fn with_scratch_ty(&mut self, reg_class: RegClass);
    fn with_scratch<T: ScratchType>(...) { self.with_scratch_ty(T::reg_class(), ...) }
}

to avoid macros? That way assemblers would implement with_scratch_ty under the hood and with_scratch as-is today would become a convenience over that method

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:11):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:12):

saulecabrera created PR review comment:

Yeah, makes sense to me, I'll update.

Wasmtime GitHub notifications bot (Jun 10 2025 at 16:12):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:11):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera has marked PR #10998 as ready for review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera requested cfallin for a review on PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera requested wasmtime-compiler-reviewers for a review on PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera requested wasmtime-core-reviewers for a review on PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:32):

saulecabrera requested pchickey for a review on PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:33):

saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:33):

saulecabrera created PR review comment:

Got rid of the macro, but ended with a slightly different version than what you proposed, mainly because I think it make sense to have the wrapper derive the register class and delegate to Masm::with_scratch<T,_>, which is what I was trying to solve for.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:34):

saulecabrera updated PR #10998.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:44):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 10 2025 at 17:44):

alexcrichton created PR review comment:

:+1:

Wasmtime GitHub notifications bot (Jun 10 2025 at 18:09):

alexcrichton merged PR #10998.

Last updated: Dec 06 2025 at 07:03 UTC