Stream: git-wasmtime

Topic: wasmtime / issue #3573 ISLE: support bool types and const...

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2021 at 18:49):

cfallin opened issue #3573:

ISLE's prototype compiler was built with the simplifying assumption that all constants are integers. "Symbolic constants" (imported opaque values) like $MYVALUE were added later. It's possible to define bool as a primitive type, but the syntax for bool constants, #t and #f, results in generated code that uses integer values 1 and 0 because... everything is (was) an integer. This is clearly suboptimal! In #3572 @alexcrichton used the clever workaround of $false, leveraging the support for arbitrary passthrough of constant names to actually get a false in the code.

We should (i) add a ConstBool alongside ConstInt in the IR (PatternInst and ExprInst), and (ii) codegen these properly as Rust bool types. We might also consider at some point making our "primitive" type hierarchy a bit richer and distinguishing ints and bools, but that's lower-priority.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2021 at 18:49):

cfallin labeled issue #3573:

ISLE's prototype compiler was built with the simplifying assumption that all constants are integers. "Symbolic constants" (imported opaque values) like $MYVALUE were added later. It's possible to define bool as a primitive type, but the syntax for bool constants, #t and #f, results in generated code that uses integer values 1 and 0 because... everything is (was) an integer. This is clearly suboptimal! In #3572 @alexcrichton used the clever workaround of $false, leveraging the support for arbitrary passthrough of constant names to actually get a false in the code.

We should (i) add a ConstBool alongside ConstInt in the IR (PatternInst and ExprInst), and (ii) codegen these properly as Rust bool types. We might also consider at some point making our "primitive" type hierarchy a bit richer and distinguishing ints and bools, but that's lower-priority.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2021 at 18:49):

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

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:

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

Learn more.
</details>

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2021 at 20:38):

fitzgen commented on issue #3573:

FWIW, we use $true and $false elsewhere, eg:

https://github.com/bytecodealliance/wasmtime/blob/0580c84405cb5808518b187d1851dc17e7538108/cranelift/codegen/src/isa/x64/lower.isle#L24-L30

Not convinced that #t and #f is really that much of an improvement.

view this post on Zulip Wasmtime GitHub notifications bot (Nov 30 2021 at 21:12):

cfallin commented on issue #3573:

Yep, we'd want to clean up said uses all at the same time.

I'm not as concerned about syntax; we could even just accept tokens true and false; my concern is more that $true and $false are abusing a feature meant for other things (externally-defined constants) and if we have bool values, we should represent them as such in the IR. Getting the type right there would also allow us to use them in the future in other DSL-compiler optimizations.

view this post on Zulip Wasmtime GitHub notifications bot (May 04 2022 at 20:02):

cfallin labeled issue #3573:

ISLE's prototype compiler was built with the simplifying assumption that all constants are integers. "Symbolic constants" (imported opaque values) like $MYVALUE were added later. It's possible to define bool as a primitive type, but the syntax for bool constants, #t and #f, results in generated code that uses integer values 1 and 0 because... everything is (was) an integer. This is clearly suboptimal! In #3572 @alexcrichton used the clever workaround of $false, leveraging the support for arbitrary passthrough of constant names to actually get a false in the code.

We should (i) add a ConstBool alongside ConstInt in the IR (PatternInst and ExprInst), and (ii) codegen these properly as Rust bool types. We might also consider at some point making our "primitive" type hierarchy a bit richer and distinguishing ints and bools, but that's lower-priority.

Last updated: Oct 23 2024 at 20:03 UTC