cranelift: Fix bint implementation on interpreter

[afonso360]

Hey,

The interpreter was returning -1 instead of 1 for positive values on the bint instruction.
This also extends the bint test suite to cover all types.

[abrown]

@afonso360, good to see you again! I think this is mostly good to go except for SIMD types: bint is a valid instruction for SIMD but it is not clear what the result should be. Should it be all ones/zeroes to match the Wasm SIMD specification for booleans? Or should it be a single one or zero (per lane) to match the CLIF instruction description? I think there is more context about this in #3205.

Because of this, I think the right thing to do here is to prevent using bint with vector types. This would mean changing the allowed operands_in in cranelift/codegen/meta/src/shared/instructions.rs. From searching through this repository, I believe bint is only ever used for scalar values so, unless some external consumer of bint is using the vector version (not implemented in any backend, I think), then this change should be a safe one to make.

Also, as a side note, could this PR close #2237? Or is that issue already effectively done?

[bjorn3]

The bint instruction returns 0 or 1. The bmask instruction returns all zeros or all ones. #3205 is about the actual representation in registers and memory, not about the result of bint/bmask, right?

[afonso360]

I didn't realize bint was also a vector instruction. I'm up for implementing it for vectors or restricting vectors from this instruction. Let me know what you would like me to do.

#3205 is about the actual representation in registers and memory, not about the result of bint/bmask, right?

That is my understanding of it as well.

Also, as a side note, could this PR close #2237? Or is that issue already effectively done?

No, that issue I think is mostly related to the trampoline generating the wrong code when loading / storing booleans, and its why I had to do duplicate some code in this PR in order to not have boolean arguments to the test cases.

[abrown]

I didn't realize bint was also a vector instruction. I'm up for implementing it for vectors or restricting vectors from this instruction. Let me know what you would like me to do.

I think restricting vectors from this instruction is the easiest path for now.

#3205 is about the actual representation in registers and memory, not about the result of bint/bmask, right?

That is my understanding of it as well.

Yes, primarily it is about representations, but @cfallin mentions bint in the description and I bring up an all ones/all zeroes comment later. The main reason for me mentioning it here is that I want GitHub to keep track of all the cross-links between related issues (and semi-related issues, in this case) so that it is easier to investigate later.

Also, as a side note, could this PR close #2237? Or is that issue already effectively done?

No, that issue I think is mostly related to the trampoline generating the wrong code when loading / storing booleans, and its why I had to do duplicate some code in this PR in order to not have boolean arguments to the test cases.

👍

[abrown]

LGTM! (@cfallin, you might want to take a look at this when you get back; we can always add back support for vector bint but until we have lowerings for this case this seemed the safest approach).