wasmtime / issue #3753 ISLE: add an implicit type convers... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / issue #3753 ISLE: add an implicit type convers...

Wasmtime GitHub notifications bot (Feb 01 2022 at 19:50):

fitzgen labeled issue #3753:

There is a lot of (RegMem.Reg my_reg) expressions in the x64 backend that convert a Reg into a RegMem when passing a Reg as an operand to an instruction that can also take a memory operand. Even more of this kind of boring conversion will be littered about with the introduction of newtypes for GPRs and XMMs in #3752 where we have to do (xmm_to_xmm_mem my_xmm).

If we were writing these APIs in Rust, we would take an reg_mem: impl Into<RegMem> param for the first example and "hide" the type conversion inside the API so that the noise is hidden from callers and the API feels nicer and more flexible. The second example would have xmm_mem: impl Into<XmmMem>. We don't have that mechanism in ISLE and the result is that certain bits of code are painfully noisy to the point where it is distracting from what instructions we are actually lowering into.

This issue is proposing that we add something similar to impl Into<T> to ISLE, to fulfill those same API ergonomics/niceties, but much simpler and without introducing a whole traits system.

First off, a big constraint: we want to maintain that there is only one expected type for every expression. This keeps type checking a single, simple pass. We don't need to do any kind of type constraint collection/solving/inference or anything like that.

Straw person proposal (feel free to bikeshed on syntax):
;; A plain decl+constructor that happens to take a `Reg` and return a `RegMem`.
(decl reg_to_reg_mem (Reg) RegMem)
(rule (reg_to_reg_mem r)
      (RegMem.Reg r))

;; Declare that we can convert a `Reg` into a `RegMem` with the
;; `reg_to_reg_mem` constructor.
;;
;; Note that this is unidirectional and does *not* imply you can
;; convert a `RegMem` into a `Reg`!
(converts Reg RegMem reg_to_reg_mem)

;; The x64 add instruction takes two operands: a register operand and
;; a register-or-memory operand.
(decl x64_add (Reg RegMem) Reg)
(rule (x64_add src1 src2) ...)

;; We can pass a `Reg` as the second register-or-memory operand to
;; `x64_add`, without explicitly writing any type conversions, and
;; the ISLE compiler will automatically insert a call to the
;; `reg_to_reg_mem` constructor for us.
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a b))

;; That is, the above is equivalent to this:
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a (reg_to_reg_mem b)))
How do we implement this? Well when you try to compile the above example with ISLE today, it gives you a type error saying that it expected a RegMem but got a Reg. It already knows the expected and actual types, so it can just look up and see if an implicit type conversion between the two is already registered. If so, then insert the conversion call. If not, then report the error.

Thoughts?

(cc @cfallin @abrown @uweigand)

Wasmtime GitHub notifications bot (Feb 01 2022 at 19:50):

fitzgen opened issue #3753:

There is a lot of (RegMem.Reg my_reg) expressions in the x64 backend that convert a Reg into a RegMem when passing a Reg as an operand to an instruction that can also take a memory operand. Even more of this kind of boring conversion will be littered about with the introduction of newtypes for GPRs and XMMs in #3752 where we have to do (xmm_to_xmm_mem my_xmm).

If we were writing these APIs in Rust, we would take an reg_mem: impl Into<RegMem> param for the first example and "hide" the type conversion inside the API so that the noise is hidden from callers and the API feels nicer and more flexible. The second example would have xmm_mem: impl Into<XmmMem>. We don't have that mechanism in ISLE and the result is that certain bits of code are painfully noisy to the point where it is distracting from what instructions we are actually lowering into.

This issue is proposing that we add something similar to impl Into<T> to ISLE, to fulfill those same API ergonomics/niceties, but much simpler and without introducing a whole traits system.

First off, a big constraint: we want to maintain that there is only one expected type for every expression. This keeps type checking a single, simple pass. We don't need to do any kind of type constraint collection/solving/inference or anything like that.

Straw person proposal (feel free to bikeshed on syntax):
;; A plain decl+constructor that happens to take a `Reg` and return a `RegMem`.
(decl reg_to_reg_mem (Reg) RegMem)
(rule (reg_to_reg_mem r)
      (RegMem.Reg r))

;; Declare that we can convert a `Reg` into a `RegMem` with the
;; `reg_to_reg_mem` constructor.
;;
;; Note that this is unidirectional and does *not* imply you can
;; convert a `RegMem` into a `Reg`!
(converts Reg RegMem reg_to_reg_mem)

;; The x64 add instruction takes two operands: a register operand and
;; a register-or-memory operand.
(decl x64_add (Reg RegMem) Reg)
(rule (x64_add src1 src2) ...)

;; We can pass a `Reg` as the second register-or-memory operand to
;; `x64_add`, without explicitly writing any type conversions, and
;; the ISLE compiler will automatically insert a call to the
;; `reg_to_reg_mem` constructor for us.
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a b))

;; That is, the above is equivalent to this:
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a (reg_to_reg_mem b)))
How do we implement this? Well when you try to compile the above example with ISLE today, it gives you a type error saying that it expected a RegMem but got a Reg. It already knows the expected and actual types, so it can just look up and see if an implicit type conversion between the two is already registered. If so, then insert the conversion call. If not, then report the error.

Thoughts?

(cc @cfallin @abrown @uweigand)

Wasmtime GitHub notifications bot (Feb 01 2022 at 19:50):

github-actions[bot] commented on issue #3753:

Subscribe to Label Action

cc @cfallin, @fitzgen

<details>
This issue or pull request has been labeled: "isle"

Thus the following users have been cc'd because of the following labels:

cfallin: isle

fitzgen: isle

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Feb 01 2022 at 19:57):

cfallin commented on issue #3753:

+1, I think this makes a ton of sense, and as we discussed it could make a lot of the tedium in the rules today (value_regs, def_inst) go away if we define right implicit conversions. So we finally reach the ultimate goal of something like
(rule (lower (iadd (imul a b) c)
         (madd a b c))
The main downside is the increased implicitness. An idea: perhaps we could have a mode where the compiler expands implicit conversions and dumps the AST? (Something like cargo expand in principle)

Wasmtime GitHub notifications bot (Feb 01 2022 at 19:57):

cfallin edited a comment on issue #3753:

+1, I think this makes a ton of sense, and as we discussed it could make a lot of the tedium in the rules today (value_regs, def_inst) go away if we define right implicit conversions. So we finally reach the ultimate goal of something like
(rule (lower (iadd (imul a b) c))
      (madd a b c))
The main downside is the increased implicitness. An idea: perhaps we could have a mode where the compiler expands implicit conversions and dumps the AST? (Something like cargo expand in principle)

Wasmtime GitHub notifications bot (Feb 01 2022 at 20:02):

fitzgen commented on issue #3753:

def_inst is actually an interesting thing to bring up because I've only thought about constructors thus far. I think we might want to have separate sets of conversions for constructors vs extractors.

Wasmtime GitHub notifications bot (Feb 01 2022 at 20:09):

cfallin commented on issue #3753:

Yep, that makes sense to me, since they're different semantic contexts (pattern vs expr). implicit_etor or etor_convert and implicit_ctor or ctor_convert perhaps?

Wasmtime GitHub notifications bot (Feb 01 2022 at 20:11):

fitzgen commented on issue #3753:

I think I would prefer constructor to ctor and extractor to etor.

Wasmtime GitHub notifications bot (Feb 02 2022 at 10:39):

uweigand commented on issue #3753:

Makes sense to me as well!

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:25):

cfallin commented on issue #3753:

Implemented in #3807.

Wasmtime GitHub notifications bot (Feb 23 2022 at 21:25):

cfallin closed issue #3753:

There is a lot of (RegMem.Reg my_reg) expressions in the x64 backend that convert a Reg into a RegMem when passing a Reg as an operand to an instruction that can also take a memory operand. Even more of this kind of boring conversion will be littered about with the introduction of newtypes for GPRs and XMMs in #3752 where we have to do (xmm_to_xmm_mem my_xmm).

If we were writing these APIs in Rust, we would take an reg_mem: impl Into<RegMem> param for the first example and "hide" the type conversion inside the API so that the noise is hidden from callers and the API feels nicer and more flexible. The second example would have xmm_mem: impl Into<XmmMem>. We don't have that mechanism in ISLE and the result is that certain bits of code are painfully noisy to the point where it is distracting from what instructions we are actually lowering into.

This issue is proposing that we add something similar to impl Into<T> to ISLE, to fulfill those same API ergonomics/niceties, but much simpler and without introducing a whole traits system.

First off, a big constraint: we want to maintain that there is only one expected type for every expression. This keeps type checking a single, simple pass. We don't need to do any kind of type constraint collection/solving/inference or anything like that.

Straw person proposal (feel free to bikeshed on syntax):
;; A plain decl+constructor that happens to take a `Reg` and return a `RegMem`.
(decl reg_to_reg_mem (Reg) RegMem)
(rule (reg_to_reg_mem r)
      (RegMem.Reg r))

;; Declare that we can convert a `Reg` into a `RegMem` with the
;; `reg_to_reg_mem` constructor.
;;
;; Note that this is unidirectional and does *not* imply you can
;; convert a `RegMem` into a `Reg`!
(converts Reg RegMem reg_to_reg_mem)

;; The x64 add instruction takes two operands: a register operand and
;; a register-or-memory operand.
(decl x64_add (Reg RegMem) Reg)
(rule (x64_add src1 src2) ...)

;; We can pass a `Reg` as the second register-or-memory operand to
;; `x64_add`, without explicitly writing any type conversions, and
;; the ISLE compiler will automatically insert a call to the
;; `reg_to_reg_mem` constructor for us.
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a b))

;; That is, the above is equivalent to this:
(let ((a Reg ...)
      (b Reg ...))
  (x64_add a (reg_to_reg_mem b)))
How do we implement this? Well when you try to compile the above example with ISLE today, it gives you a type error saying that it expected a RegMem but got a Reg. It already knows the expected and actual types, so it can just look up and see if an implicit type conversion between the two is already registered. If so, then insert the conversion call. If not, then report the error.

Thoughts?

(cc @cfallin @abrown @uweigand)

Last updated: Dec 23 2024 at 12:05 UTC