From 1970299926dd6d8406e132501694c37d3131964d Mon Sep 17 00:00:00 2001 From: Jacob Bramley Date: Fri, 19 May 2023 16:27:05 +0100 Subject: [PATCH] Remove unnecessary unsafety in intrinsic tests. This fixes "unnecessary `unsafe` block" warnings encountered when building the generated rust_programs. The only pattern that actually required `unsafe` was transmuting bit patterns into floats. This patch uses the safe `from_bits` instead, but because that isn't const, we have to make them local let-bound variables. --- .../crates/intrinsic-test/src/argument.rs | 42 +++++++- .../stdarch/crates/intrinsic-test/src/main.rs | 3 +- .../crates/intrinsic-test/src/types.rs | 98 ++++++++----------- 3 files changed, 79 insertions(+), 64 deletions(-) diff --git a/library/stdarch/crates/intrinsic-test/src/argument.rs b/library/stdarch/crates/intrinsic-test/src/argument.rs index dd930115b162..7dbbb5a2a531 100644 --- a/library/stdarch/crates/intrinsic-test/src/argument.rs +++ b/library/stdarch/crates/intrinsic-test/src/argument.rs @@ -92,6 +92,37 @@ impl Argument { constraints: constraint.map_or(vec![], |r| vec![r]), } } + + fn is_rust_vals_array_const(&self) -> bool { + use TypeKind::*; + match self.ty { + // Floats have to be loaded at runtime for stable NaN conversion. + IntrinsicType::Type { kind: Float, .. } => false, + IntrinsicType::Type { + kind: Int | UInt | Poly, + .. + } => true, + _ => unimplemented!(), + } + } + + /// The binding keyword (e.g. "const" or "let") for the array of possible test inputs. + pub fn rust_vals_array_binding(&self) -> impl std::fmt::Display { + if self.is_rust_vals_array_const() { + "const" + } else { + "let" + } + } + + /// The name (e.g. "A_VALS" or "a_vals") for the array of possible test inputs. + pub fn rust_vals_array_name(&self) -> impl std::fmt::Display { + if self.is_rust_vals_array_const() { + format!("{}_VALS", self.name.to_uppercase()) + } else { + format!("{}_vals", self.name.to_lowercase()) + } + } } #[derive(Debug, PartialEq, Clone)] @@ -143,7 +174,7 @@ impl ArgumentList { .filter_map(|arg| { (!arg.has_constraint()).then(|| { format!( - "const {ty} {name}_vals[] = {{ {values} }};", + "const {ty} {name}_vals[] = {values};", ty = arg.ty.c_scalar_type(), name = arg.name, values = arg.ty.populate_random(loads, &Language::C) @@ -161,8 +192,9 @@ impl ArgumentList { .filter_map(|arg| { (!arg.has_constraint()).then(|| { format!( - "const {upper_name}_VALS: [{ty}; {load_size}] = unsafe{{ [{values}] }};", - upper_name = arg.name.to_uppercase(), + "{bind} {name}: [{ty}; {load_size}] = {values};", + bind = arg.rust_vals_array_binding(), + name = arg.rust_vals_array_name(), ty = arg.ty.rust_scalar_type(), load_size = arg.ty.num_lanes() * arg.ty.num_vectors() + loads - 1, values = arg.ty.populate_random(loads, &Language::Rust) @@ -222,9 +254,9 @@ impl ArgumentList { .filter_map(|arg| { (!arg.has_constraint()).then(|| { format!( - "let {name} = {load}({upper_name}_VALS.as_ptr().offset(i));", + "let {name} = {load}({vals_name}.as_ptr().offset(i));", name = arg.name, - upper_name = arg.name.to_uppercase(), + vals_name = arg.rust_vals_array_name(), load = if arg.is_simd() { arg.ty.get_load_function(false) } else { diff --git a/library/stdarch/crates/intrinsic-test/src/main.rs b/library/stdarch/crates/intrinsic-test/src/main.rs index 3c181584beff..6b4d19d898ae 100644 --- a/library/stdarch/crates/intrinsic-test/src/main.rs +++ b/library/stdarch/crates/intrinsic-test/src/main.rs @@ -178,9 +178,8 @@ fn generate_rust_program(notices: &str, intrinsic: &Intrinsic, a32: bool) -> Str #![allow(non_upper_case_globals)] use core_arch::arch::{target_arch}::*; -{arglists} - fn main() {{ +{arglists} {passes} }} "#, diff --git a/library/stdarch/crates/intrinsic-test/src/types.rs b/library/stdarch/crates/intrinsic-test/src/types.rs index 0e8bbb112547..dd27949dd88b 100644 --- a/library/stdarch/crates/intrinsic-test/src/types.rs +++ b/library/stdarch/crates/intrinsic-test/src/types.rs @@ -1,6 +1,8 @@ use std::fmt; use std::str::FromStr; +use itertools::Itertools as _; + use crate::values::value_for_array; use crate::Language; @@ -288,82 +290,64 @@ impl IntrinsicType { } } - /// Generates a comma list of values that can be used to initialize the array that - /// an argument for the intrinsic call is loaded from. + /// Generates an initialiser for an array, which can be used to initialise an argument for the + /// intrinsic call. + /// /// This is determistic based on the pass number. /// /// * `loads`: The number of values that need to be loaded from the argument array /// * e.g for argument type uint32x2, loads=2 results in a string representing 4 32-bit values /// /// Returns a string such as - /// * `0x1, 0x7F, 0xFF` if `language` is `Language::C` - /// * `0x1 as _, 0x7F as _, 0xFF as _` if `language` is `Language::Rust` + /// * `{0x1, 0x7F, 0xFF}` if `language` is `Language::C` + /// * `[0x1 as _, 0x7F as _, 0xFF as _]` if `language` is `Language::Rust` pub fn populate_random(&self, loads: u32, language: &Language) -> String { match self { IntrinsicType::Ptr { child, .. } => child.populate_random(loads, language), IntrinsicType::Type { bit_len: Some(bit_len), - kind, + kind: TypeKind::Int | TypeKind::UInt | TypeKind::Poly, simd_len, vec_len, .. - } if kind == &TypeKind::Int || kind == &TypeKind::UInt || kind == &TypeKind::Poly => (0 - ..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1)) - .map(|i| { - format!( - "{}{}", - value_for_array(*bit_len, i), - match language { - &Language::Rust => format!(" as {ty} ", ty = self.rust_scalar_type()), - &Language::C => String::from(""), - } - ) - }) - .collect::>() - .join(","), + } => { + let (prefix, as_type, suffix) = match language { + &Language::Rust => ("[", format!(" as {}", self.rust_scalar_type()), "]"), + &Language::C => ("{", "".into(), "}"), + }; + format!( + "{prefix}{body}{suffix}", + body = (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1)) + .format_with(", ", |i, fmt| fmt(&format_args!( + "{src}{as_type}", + src = value_for_array(*bit_len, i) + ))) + ) + } IntrinsicType::Type { kind: TypeKind::Float, - bit_len: Some(32), + bit_len: Some(bit_len @ (32 | 64)), simd_len, vec_len, .. - } => (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1)) - .map(|i| { - format!( - "{}({})", - match language { - &Language::Rust => "std::mem::transmute", - &Language::C => "cast", - }, - value_for_array(32, i), - ) - }) - .collect::>() - .join(","), - IntrinsicType::Type { - kind: TypeKind::Float, - bit_len: Some(64), - simd_len, - vec_len, - .. - } => (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1)) - .map(|i| { - format!( - "{}({}{})", - match language { - &Language::Rust => "std::mem::transmute", - &Language::C => "cast", - }, - value_for_array(64, i), - match language { - &Language::Rust => " as u64", - &Language::C => "", - } - ) - }) - .collect::>() - .join(","), - _ => unreachable!("populate random: {:#?}", self), + } => { + let (prefix, cast_prefix, cast_suffix, suffix) = match (language, bit_len) { + (&Language::Rust, 32) => ("[", "f32::from_bits(", ")", "]"), + (&Language::Rust, 64) => ("[", "f64::from_bits(", ")", "]"), + (&Language::C, 32) => ("{", "cast(", ")", "}"), + (&Language::C, 64) => ("{", "cast(", ")", "}"), + _ => unreachable!(), + }; + format!( + "{prefix}{body}{suffix}", + body = (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1)) + .format_with(", ", |i, fmt| fmt(&format_args!( + "{cast_prefix}{src}{cast_suffix}", + src = value_for_array(*bit_len, i) + ))) + ) + } + _ => unimplemented!("populate random: {:#?}", self), } }