Stream: git-wasmtime

Topic: wasmtime / PR #9504 x64: simplify `SyntheticAmode::finalize`

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:24):

abrown opened PR #9504 from abrown:refactor-amode to bytecodealliance:main:

There is no need for a SyntheticAmode to request mutable access to the entire EmitState; it only needs a read-only FrameLayout.

<!--
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 (Oct 23 2024 at 22:24):

abrown requested elliottt for a review on PR #9504.

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:24):

abrown requested wasmtime-compiler-reviewers for a review on PR #9504.

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:27):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:27):

cfallin has enabled auto merge for PR #9504.

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:28):

abrown commented on PR #9504:

What I'm aiming at is allowing any new assembler code to be ignorant of as much of the internals of cranelift-codegen as possible. I think this will make it easier to test, etc. It really would be nice if the assembler didn't need to know about SyntheticAmode at all; if a SyntheticAmode could be finalized down into its equivalent Amode before the assembler ever sees it, that would be a good simplification. But all of the GprMem types and such point at SyntheticAmodes...

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:30):

abrown updated PR #9504.

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:31):

abrown commented on PR #9504:

Oh, sorry, had to fix some build issues so I force-pushed!

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:32):

cfallin commented on PR #9504:

Interesting; in an old version of the MachInst backend, there was actually a separate pass to rewrite SyntheticAmodes, but we try to avoid separate passes and I don't think we have info to lower them when generating the instructions because they depend on details only known after regalloc, like frame layout. Perhaps one could design a way with parameterization of the enum (e.g. Inst<A: Into<Amode>>) and then a method to "lift" the into to Inst (Haskell brain: I want fmap)?

Wasmtime GitHub notifications bot (Oct 23 2024 at 22:57):

cfallin merged PR #9504.

Last updated: Nov 22 2024 at 16:03 UTC