T0b1/wasmtime
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix masking of vector shift values

Browse Source 
Previously, the logic was wrong on two counts:
 - It used the bits of the entire vector (e.g. i32x4 -> 128) instead of just the lane bits (e.g. i32x4 -> 32).
 - It used the type of the first operand before it was bitcast to its correct type. Remember that, by default, vectors are handed around as i8x16 and we must bitcast them to their correct type for Cranelift's verifier; see https://github.com/bytecodealliance/wasmtime/issues/1147 for discussion on this. This fix simply uses the type of the instruction itself, which is equivalent and hopefully less fragile to any changes.
This commit is contained in:
Andrew Brown
2020-05-02 15:03:40 -07:00
parent 59039df001
commit 4155d15e69
1 changed files with 4 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
cranelift/wasm/src/code_translator.rs
View File

@@ -1402,7 +1402,7 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
Operator::I8x16Shl | Operator::I16x8Shl | Operator::I32x4Shl | Operator::I64x2Shl => { Operator::I8x16Shl | Operator::I16x8Shl | Operator::I32x4Shl | Operator::I64x2Shl => {
let (a, b) = state.pop2(); let (a, b) = state.pop2();
let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder); let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder);
let bitwidth = i64::from(builder.func.dfg.value_type(a).bits()); let bitwidth = i64::from(type_of(op).lane_bits());
// The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width // The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width
// we do `b AND 15`; this means fewer instructions than `iconst + urem`. // we do `b AND 15`; this means fewer instructions than `iconst + urem`.
let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1); let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1);
@@ -1411,16 +1411,16 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
Operator::I8x16ShrU | Operator::I16x8ShrU | Operator::I32x4ShrU | Operator::I64x2ShrU => { Operator::I8x16ShrU | Operator::I16x8ShrU | Operator::I32x4ShrU | Operator::I64x2ShrU => {
let (a, b) = state.pop2(); let (a, b) = state.pop2();
let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder); let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder);
let bitwidth = i64::from(builder.func.dfg.value_type(a).bits()); let bitwidth = i64::from(type_of(op).lane_bits());
// The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width // The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width
// we do `b AND 15`; this means fewer instructions than `iconst + urem`. // we do `b AND 15`; this means fewer instructions than `iconst + urem`.
let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1); let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1);
state.push1(builder.ins().ushr(bitcast_a, b_mod_bitwidth)) state.push1(builder.ins().ushr(bitcast_a, b_mod_bitwidth))
} }
Operator::I8x16ShrS | Operator::I16x8ShrS | Operator::I32x4ShrS => { Operator::I8x16ShrS | Operator::I16x8ShrS | Operator::I32x4ShrS | Operator::I64x2ShrS => {
let (a, b) = state.pop2(); let (a, b) = state.pop2();
let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder); let bitcast_a = optionally_bitcast_vector(a, type_of(op), builder);
let bitwidth = i64::from(builder.func.dfg.value_type(a).bits()); let bitwidth = i64::from(type_of(op).lane_bits());
// The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width // The spec expects to shift with `b mod lanewidth`; so, e.g., for 16 bit lane-width
// we do `b AND 15`; this means fewer instructions than `iconst + urem`. // we do `b AND 15`; this means fewer instructions than `iconst + urem`.
let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1); let b_mod_bitwidth = builder.ins().band_imm(b, bitwidth - 1);
@@ -1544,7 +1544,6 @@ pub fn translate_operator<FE: FuncEnvironment + ?Sized>(
} }
Operator::I8x16Mul Operator::I8x16Mul
| Operator::I64x2Mul | Operator::I64x2Mul
| Operator::I64x2ShrS
| Operator::I32x4TruncSatF32x4S | Operator::I32x4TruncSatF32x4S
| Operator::I32x4TruncSatF32x4U | Operator::I32x4TruncSatF32x4U
| Operator::I64x2TruncSatF64x2S | Operator::I64x2TruncSatF64x2S