From c475735f5e4bf55bd99225304bde441365a65741 Mon Sep 17 00:00:00 2001 From: Jan-Justin van Tonder Date: Tue, 4 Apr 2023 14:14:16 +0200 Subject: [PATCH] cranelift-interpreter: Fix incorrect scalar_to_vector result (#6133) * The `vectorizelanes` function performs a check to see whether there is a single value provided in an array, and if so returns it as a scalar. While elsewhere in the interpreter this behaviour is relied upon, it yields an incorrect result when attempting to convert a scalar to a vector. The original `vectorizelanes` remains untouched, however, an unconditional variant `vectorizelanes_all` was added. * A test was added under `filetests/runtests/issue5911.clif`. Fixes #5911 --- .../runtests/simd-scalartovector-aarch64.clif | 2 ++ .../runtests/simd-scalartovector.clif | 2 ++ cranelift/interpreter/src/step.rs | 27 ++++++++++++------- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/cranelift/filetests/filetests/runtests/simd-scalartovector-aarch64.clif b/cranelift/filetests/filetests/runtests/simd-scalartovector-aarch64.clif index a49b525260..0721599a21 100644 --- a/cranelift/filetests/filetests/runtests/simd-scalartovector-aarch64.clif +++ b/cranelift/filetests/filetests/runtests/simd-scalartovector-aarch64.clif @@ -1,4 +1,5 @@ test run +test interpret target aarch64 target s390x ; i8 and i16 are invalid source sizes for x86_64 @@ -16,5 +17,6 @@ block0(v0: i16): v1 = scalar_to_vector.i16x8 v0 return v1 } +; run: %scalartovector_i16(0) == [0 0 0 0 0 0 0 0] ; run: %scalartovector_i16(1) == [1 0 0 0 0 0 0 0] ; run: %scalartovector_i16(65535) == [65535 0 0 0 0 0 0 0] diff --git a/cranelift/filetests/filetests/runtests/simd-scalartovector.clif b/cranelift/filetests/filetests/runtests/simd-scalartovector.clif index 5144c81113..7891693f3f 100644 --- a/cranelift/filetests/filetests/runtests/simd-scalartovector.clif +++ b/cranelift/filetests/filetests/runtests/simd-scalartovector.clif @@ -1,4 +1,5 @@ test run +test interpret target aarch64 target s390x set enable_simd @@ -18,6 +19,7 @@ block0(v0: i64): v1 = scalar_to_vector.i64x2 v0 return v1 } +; run: %scalartovector_i64(0) == [0 0] ; run: %scalartovector_i64(1) == [1 0] ; run: %scalartovector_i64(18446744073709551615) == [18446744073709551615 0] diff --git a/cranelift/interpreter/src/step.rs b/cranelift/interpreter/src/step.rs index 187413ac4d..897b6e0452 100644 --- a/cranelift/interpreter/src/step.rs +++ b/cranelift/interpreter/src/step.rs @@ -934,14 +934,15 @@ where Opcode::Bitcast | Opcode::ScalarToVector => { let input_ty = inst_context.type_of(inst_context.args()[0]).unwrap(); let arg0 = extractlanes(&arg(0)?, input_ty)?; - - assign(vectorizelanes( - &arg0 - .into_iter() - .map(|x| V::convert(x, ValueConversionKind::Exact(ctrl_ty.lane_type()))) - .collect::>>()?, - ctrl_ty, - )?) + let lanes = &arg0 + .into_iter() + .map(|x| V::convert(x, ValueConversionKind::Exact(ctrl_ty.lane_type()))) + .collect::>>()?; + assign(match inst.opcode() { + Opcode::Bitcast => vectorizelanes(lanes, ctrl_ty)?, + Opcode::ScalarToVector => vectorizelanes_all(lanes, ctrl_ty)?, + _ => unreachable!(), + }) } Opcode::Ireduce => assign(Value::convert( arg(0)?, @@ -1552,9 +1553,17 @@ where { // If the array is only one element, return it as a scalar. if x.len() == 1 { - return Ok(x[0].clone()); + Ok(x[0].clone()) + } else { + vectorizelanes_all(x, vector_type) } +} +/// Convert a Rust array of [Value] back into a `Value::vector`. +fn vectorizelanes_all(x: &[V], vector_type: types::Type) -> ValueResult +where + V: Value, +{ let lane_type = vector_type.lane_type(); let iterations = match lane_type { types::I8 => 1,