Stream: git-wasmtime

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


view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:35):

cfallin submitted PR review.

view this post on Zulip 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 :-)

view this post on Zulip 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.

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

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:36):

elliottt submitted PR review.

view this post on Zulip 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 :)

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:39):

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

view this post on Zulip 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:

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:40):

elliottt submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin submitted PR review.

view this post on Zulip 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!

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 20:42):

cfallin has enabled auto merge for PR #4562.

view this post on Zulip Wasmtime GitHub notifications bot (Aug 01 2022 at 21:10):

cfallin merged PR #4562.


Last updated: Dec 23 2024 at 12:05 UTC