Stream: git-wasmtime

Topic: wasmtime / PR #5432 Simplify LowerBackend interface

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:41):

uweigand opened PR #5432 from simplify-lowerbackend to main:

Refactor lower_branch to have Unit result

Branches cannot have any output, so it is more straightforward to have the ISLE term return Unit instead of InstOutput.

Also provide a new emit_side_effect term to simplify implementation of lower_branch rules with Unit result.

Simplify LowerBackend interface

Move all remaining asserts from the LowerBackend::lower and ::lower_branch_group into the common call site.

Change return value of ::lower to Option<InstOutput>, and return value of ::lower_branch_group to Option<()> to match ISLE term signature.

Only pass the first branch into ::lower_branch_group and rename it to ::lower_branch.

As a result of all those changes, LowerBackend routines now consists solely to calls to the corresponding ISLE routines.

CC @cfallin @jameysharp @bjorn3 - as discussed here https://github.com/bytecodealliance/wasmtime/pull/5429#issuecomment-1349911589.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:48):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:48):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:48):

cfallin created PR review comment:

The CodegenResult return type was previously self-documenting, but the Option now is a little ambiguous. Can we add a note here that returning None indicates that a lowering is missing for the given instruction (and likewise below for lower_branch)?

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:54):

uweigand updated PR #5432 from simplify-lowerbackend to main.

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:55):

uweigand submitted PR review.

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:55):

uweigand created PR review comment:

Makes sense; I've added this now.

Wasmtime GitHub notifications bot (Dec 13 2022 at 23:55):

cfallin has enabled auto merge for PR #5432.

Wasmtime GitHub notifications bot (Dec 14 2022 at 00:48):

cfallin merged PR #5432.

Last updated: Dec 23 2024 at 12:05 UTC