Stream: git-wasmtime

Topic: wasmtime / PR #4562 ISLE: Allow shadowing in let expressions

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:31):

elliottt edited PR #4562 from trevor/isle-wildcard-let to main:

Currently, a let expression in ISLE currently requires unique names for each declaration. If you are sequencing a number of effects together using a let expression, you'll end up with multiple declarations whose results you don't need. Here's an example of this from the s390x backend: https://github.com/bytecodealliance/wasmtime/blob/8e9e9c52a1da9c661ad8a25210e38826bf8f7362/cranelift/codegen/src/isa/s390x/inst.isle#L3057-L3064

This PR allows multiple definitions with the name _ in a let expression, removing the need to come up with unique names. The previous example can be refactored to the following after this change:

;; Use a boolean condition to select between two registers.
(decl select_bool_reg (Type ProducesBool Reg Reg) Reg)
(rule (select_bool_reg ty (ProducesBool.ProducesBool producer cond) reg_true reg_false)
      (let ((dst WritableReg (temp_writable_reg ty))
            (_ Unit (emit_producer producer))
            (_ Unit (emit_mov ty dst reg_false))
            (_ Unit (emit_consumer (emit_cmov_reg ty dst cond reg_true))))
        dst))

<!--

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 (Aug 01 2022 at 20:35):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:35):

cfallin created PR review comment:

Ah, I think you may need to rebase -- as of Friday there is now a notion of a run-test already and a separate isle_examples/run directory :-)

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:35):

elliottt edited PR #4562 from trevor/isle-wildcard-let to main:

Currently, a let expression in ISLE currently requires unique names for each declaration. If you are sequencing a number of effects together using a let expression, you'll end up with multiple declarations whose results you don't need. Here's an example of this from the s390x backend: https://github.com/bytecodealliance/wasmtime/blob/8e9e9c52a1da9c661ad8a25210e38826bf8f7362/cranelift/codegen/src/isa/s390x/inst.isle#L3057-L3064

This PR allows shadowing in let expressions, enabling the previous example to be refactored in the following way:

;; Use a boolean condition to select between two registers.
(decl select_bool_reg (Type ProducesBool Reg Reg) Reg)
(rule (select_bool_reg ty (ProducesBool.ProducesBool producer cond) reg_true reg_false)
      (let ((dst WritableReg (temp_writable_reg ty))
            (_ Unit (emit_producer producer))
            (_ Unit (emit_mov ty dst reg_false))
            (_ Unit (emit_consumer (emit_cmov_reg ty dst cond reg_true))))
        dst))

<!--

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 (Aug 01 2022 at 20:36):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:36):

elliottt created PR review comment:

It's hilarious that this is what the PR boils down to :)

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:39):

elliottt updated PR #4562 from trevor/isle-wildcard-let to main.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:40):

elliottt created PR review comment:

Nice! I rebased and removed the changes I introduced to run the link tests :+1:

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:40):

elliottt submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin created PR review comment:

Something about journeys vs. destinations etc etc ... but, yes, I'm happy it was a simple change!

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin has enabled auto merge for PR #4562.

Wasmtime GitHub notifications bot (Aug 01 2022 at 21:10):

cfallin merged PR #4562.

Last updated: Nov 22 2024 at 17:03 UTC