Stream: git-wasmtime

Topic: wasmtime / issue #5031 Cranelift: Remove booleans

Wasmtime GitHub notifications bot (Oct 07 2022 at 15:35):

afonso360 commented on issue #5031:

I'm really happy that we're finally doing this! Thank you for taking this on.

Some of our lowerings depend on the fact that bools can only be represented as all ones or all zeroes, and so now that we are removing that assumption we might want to revisit them.

The AArch64 implementation for bmask is wrong if we want to accept any integer other than -1 and 0. (i.e. 2 will be false, but should be true)

bint is also assuming that inputs are all ones or all zeros and is implemented as and_imm on AArch64.

Wasmtime GitHub notifications bot (Oct 07 2022 at 15:36):

afonso360 edited a comment on issue #5031:

I'm really happy that we're finally doing this! Thank you for taking this on.

Some of our lowerings depend on the fact that bools can only be represented as all ones or all zeroes, and so now that we are removing that assumption we might want to revisit them.

The AArch64 implementation for bmask is wrong if we want to accept any integer other than -1 and 0.

bint is also assuming that inputs are all ones or all zeros and is implemented as and_imm on AArch64. (i.e. 2 will be false, but should be true)

Wasmtime GitHub notifications bot (Oct 07 2022 at 15:38):

afonso360 edited a comment on issue #5031:

I'm really happy that we're finally doing this! Thank you for taking this on.

Some of our lowerings depend on the fact that bools can only be represented as all ones or all zeroes, and so now that we are removing that assumption we might want to revisit them.

The AArch64 implementation for bmask is wrong if we want to accept any integer other than -1 and 0. I would expect to get -1 if I do a bmask on a 2, but that's not what's going to happen. (I haven't tested this, but that's my reading of the implementation).

bint is also assuming that inputs are all ones or all zeros and is implemented as and_imm on AArch64. (i.e. 2 will be false, but should be true)

Wasmtime GitHub notifications bot (Oct 07 2022 at 16:44):

afonso360 edited a comment on issue #5031:

I'm really happy that we're finally doing this! Thank you for taking this on.

Some of our lowerings depend on the fact that bools can only be represented as all ones or all zeroes, and so now that we are removing that assumption we might want to revisit them.

The AArch64 implementation for bmask is wrong if we want to accept any integer other than -1 and 0. I would expect to get -1 if I do a bmask on a 2, but that's not what's going to happen. (I haven't tested this, but that's my reading of the implementation).

bint is also assuming that inputs are all ones or all zeros and is implemented as and_imm on AArch64. (i.e. 2 will be false, but should be true)

Edit: AArch64 has instructions (cset and csetm) that represent these two operations, so we may want to partially revert b8f6d53a6bdf861e66093a046a081c886546cdf1 to get them back

Wasmtime GitHub notifications bot (Oct 07 2022 at 23:40):

cfallin commented on issue #5031:

Also, just to note it here: if at all possible, I'd prefer for us to wait to merge this until after my mid-end work in #4953 is merged, if only because they're both large and likely-to-conflict PRs and mine has been through ~three painful rebases already :-)

Last updated: Nov 22 2024 at 17:03 UTC