wasm->CLIF translation: consistently bitcast V128 values that are blo…

[julian-seward1]

…ck formal parameters.

In the current translation of wasm (128-bit) SIMD into CLIF, we work around differences in the
type system models of wasm vs CLIF by inserting bitcast (no-op casts) CLIF instructions before
more or less every use of a SIMD value. Unfortunately this was not being done consistently and
even small examples with a single if-then-else diamond that produces a SIMD value, could cause a
verification failure downstream. In this case, the jump out of the "else" block needed a
bitcast, but didn't have one.

This patch wraps creation of CLIF jumps and conditional branches up into a pair of functions,
canonicalise_then_jump and canonicalise_then_br_z_or_nz, and uses them consistently. They
first cast the relevant block formal parameters, then generate the relevant kind of branch/jump.
Hence, provided they are also used consistently in future to generate branches/jumps in this
file, we are protected against such failures.

The patch also adds a large(ish) comment at the top explaining this in more detail.

[yurydelendik]

I wonder if we can wrap SmallVec canonicalised initialization logic into function and do it only when needed. Also, I think it is possible to avoid canonicalise_then_XXXX functions.

fn canonicalise<'a>(
    builder: &mut FunctionBuilder,
    values: &'a [ir::Value],
) -> impl AsRef<[ir::Value]> + 'a {
    enum Result<'a> {
        Ref(&'a [ir::Value]),
        Vec(SmallVec<[ir::Value; 16]>),
    }
    impl<'a> AsRef<[u8]> for Result<'a> {
        fn as_ref(&self) -> &[u8] {
            match self {
                Result::Ref(r) => r,
                Result::Vec(v) => v.as_slice(),
            }
        }    
    }
    
    // do_checks_and_stuff

    if ... {
        Result::Vec(canonicalised)
    } else {
        Result::Ref(values)
    }
}

// do e.g.
let params = canonicalise(builder, destination_args);
builder
  .ins()
  .jump(real_dest_block, params.as_ref());

It is nice to see fast path for canonicalise when SIMD is not in use too.

[abrown]

Could you create tests for this in tests/wasm or cranelift/wasmtests? I think the reason that jumps/branches weren't covered when I had to add the original bitcasts is that I believed the spec testsuite covered these cases... but apparently not.

[julian-seward1]

Thanks all for the various comments/suggestions. I pushed a revised patch that addresses all of them.