Stream: git-wasmtime

Topic: wasmtime / PR #4562 Allow multiple `_` definitions in a `...


view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2022 at 23:31):

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

<!--

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 (Jul 29 2022 at 23:36):

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

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2022 at 23:44):

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

view this post on Zulip Wasmtime GitHub notifications bot (Jul 29 2022 at 23:53):

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

On main 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 name you don't care about, like in this example 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 (Jul 29 2022 at 23:53):

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 name you don't care about, like in this example 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 (Jul 29 2022 at 23:54):

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 16:24):

elliottt has marked PR #4562 as ready for review.

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

cfallin submitted PR review.

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

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

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

elliottt requested cfallin for a review on PR #4562.

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

cfallin submitted PR review.

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

cfallin submitted PR review.

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

cfallin created PR review comment:

Would you mind turning at least this case into a run-test and invoking the rule from a test toplevel, asserting that the most recent binding wins?

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

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:26):

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:28):

elliottt submitted PR review.

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

elliottt created PR review comment:

I reworked it as a link test in the isle/isle_examples/link directory, and modified the test runner to also execute the output of rustc on these tests.


Last updated: Dec 23 2024 at 13:07 UTC