Skip to content

Commit 6e7965d

Browse files
jacobbramleyAmanieu
authored andcommitted
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.
1 parent e9100bc commit 6e7965d

File tree

3 files changed

+79
-64
lines changed

3 files changed

+79
-64
lines changed

crates/intrinsic-test/src/argument.rs

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,37 @@ impl Argument {
9292
constraints: constraint.map_or(vec![], |r| vec![r]),
9393
}
9494
}
95+
96+
fn is_rust_vals_array_const(&self) -> bool {
97+
use TypeKind::*;
98+
match self.ty {
99+
// Floats have to be loaded at runtime for stable NaN conversion.
100+
IntrinsicType::Type { kind: Float, .. } => false,
101+
IntrinsicType::Type {
102+
kind: Int | UInt | Poly,
103+
..
104+
} => true,
105+
_ => unimplemented!(),
106+
}
107+
}
108+
109+
/// The binding keyword (e.g. "const" or "let") for the array of possible test inputs.
110+
pub fn rust_vals_array_binding(&self) -> impl std::fmt::Display {
111+
if self.is_rust_vals_array_const() {
112+
"const"
113+
} else {
114+
"let"
115+
}
116+
}
117+
118+
/// The name (e.g. "A_VALS" or "a_vals") for the array of possible test inputs.
119+
pub fn rust_vals_array_name(&self) -> impl std::fmt::Display {
120+
if self.is_rust_vals_array_const() {
121+
format!("{}_VALS", self.name.to_uppercase())
122+
} else {
123+
format!("{}_vals", self.name.to_lowercase())
124+
}
125+
}
95126
}
96127

97128
#[derive(Debug, PartialEq, Clone)]
@@ -143,7 +174,7 @@ impl ArgumentList {
143174
.filter_map(|arg| {
144175
(!arg.has_constraint()).then(|| {
145176
format!(
146-
"const {ty} {name}_vals[] = {{ {values} }};",
177+
"const {ty} {name}_vals[] = {values};",
147178
ty = arg.ty.c_scalar_type(),
148179
name = arg.name,
149180
values = arg.ty.populate_random(loads, &Language::C)
@@ -161,8 +192,9 @@ impl ArgumentList {
161192
.filter_map(|arg| {
162193
(!arg.has_constraint()).then(|| {
163194
format!(
164-
"const {upper_name}_VALS: [{ty}; {load_size}] = unsafe{{ [{values}] }};",
165-
upper_name = arg.name.to_uppercase(),
195+
"{bind} {name}: [{ty}; {load_size}] = {values};",
196+
bind = arg.rust_vals_array_binding(),
197+
name = arg.rust_vals_array_name(),
166198
ty = arg.ty.rust_scalar_type(),
167199
load_size = arg.ty.num_lanes() * arg.ty.num_vectors() + loads - 1,
168200
values = arg.ty.populate_random(loads, &Language::Rust)
@@ -222,9 +254,9 @@ impl ArgumentList {
222254
.filter_map(|arg| {
223255
(!arg.has_constraint()).then(|| {
224256
format!(
225-
"let {name} = {load}({upper_name}_VALS.as_ptr().offset(i));",
257+
"let {name} = {load}({vals_name}.as_ptr().offset(i));",
226258
name = arg.name,
227-
upper_name = arg.name.to_uppercase(),
259+
vals_name = arg.rust_vals_array_name(),
228260
load = if arg.is_simd() {
229261
arg.ty.get_load_function(false)
230262
} else {

crates/intrinsic-test/src/main.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,8 @@ fn generate_rust_program(notices: &str, intrinsic: &Intrinsic, a32: bool) -> Str
178178
#![allow(non_upper_case_globals)]
179179
use core_arch::arch::{target_arch}::*;
180180
181-
{arglists}
182-
183181
fn main() {{
182+
{arglists}
184183
{passes}
185184
}}
186185
"#,

crates/intrinsic-test/src/types.rs

Lines changed: 41 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::fmt;
22
use std::str::FromStr;
33

4+
use itertools::Itertools as _;
5+
46
use crate::values::value_for_array;
57
use crate::Language;
68

@@ -288,82 +290,64 @@ impl IntrinsicType {
288290
}
289291
}
290292

291-
/// Generates a comma list of values that can be used to initialize the array that
292-
/// an argument for the intrinsic call is loaded from.
293+
/// Generates an initialiser for an array, which can be used to initialise an argument for the
294+
/// intrinsic call.
295+
///
293296
/// This is determistic based on the pass number.
294297
///
295298
/// * `loads`: The number of values that need to be loaded from the argument array
296299
/// * e.g for argument type uint32x2, loads=2 results in a string representing 4 32-bit values
297300
///
298301
/// Returns a string such as
299-
/// * `0x1, 0x7F, 0xFF` if `language` is `Language::C`
300-
/// * `0x1 as _, 0x7F as _, 0xFF as _` if `language` is `Language::Rust`
302+
/// * `{0x1, 0x7F, 0xFF}` if `language` is `Language::C`
303+
/// * `[0x1 as _, 0x7F as _, 0xFF as _]` if `language` is `Language::Rust`
301304
pub fn populate_random(&self, loads: u32, language: &Language) -> String {
302305
match self {
303306
IntrinsicType::Ptr { child, .. } => child.populate_random(loads, language),
304307
IntrinsicType::Type {
305308
bit_len: Some(bit_len),
306-
kind,
309+
kind: TypeKind::Int | TypeKind::UInt | TypeKind::Poly,
307310
simd_len,
308311
vec_len,
309312
..
310-
} if kind == &TypeKind::Int || kind == &TypeKind::UInt || kind == &TypeKind::Poly => (0
311-
..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1))
312-
.map(|i| {
313-
format!(
314-
"{}{}",
315-
value_for_array(*bit_len, i),
316-
match language {
317-
&Language::Rust => format!(" as {ty} ", ty = self.rust_scalar_type()),
318-
&Language::C => String::from(""),
319-
}
320-
)
321-
})
322-
.collect::<Vec<_>>()
323-
.join(","),
324-
IntrinsicType::Type {
325-
kind: TypeKind::Float,
326-
bit_len: Some(32),
327-
simd_len,
328-
vec_len,
329-
..
330-
} => (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1))
331-
.map(|i| {
332-
format!(
333-
"{}({})",
334-
match language {
335-
&Language::Rust => "std::mem::transmute",
336-
&Language::C => "cast<float, uint32_t>",
337-
},
338-
value_for_array(32, i),
339-
)
340-
})
341-
.collect::<Vec<_>>()
342-
.join(","),
313+
} => {
314+
let (prefix, as_type, suffix) = match language {
315+
&Language::Rust => ("[", format!(" as {}", self.rust_scalar_type()), "]"),
316+
&Language::C => ("{", "".into(), "}"),
317+
};
318+
format!(
319+
"{prefix}{body}{suffix}",
320+
body = (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1))
321+
.format_with(", ", |i, fmt| fmt(&format_args!(
322+
"{src}{as_type}",
323+
src = value_for_array(*bit_len, i)
324+
)))
325+
)
326+
}
343327
IntrinsicType::Type {
344328
kind: TypeKind::Float,
345-
bit_len: Some(64),
329+
bit_len: Some(bit_len @ (32 | 64)),
346330
simd_len,
347331
vec_len,
348332
..
349-
} => (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1))
350-
.map(|i| {
351-
format!(
352-
"{}({}{})",
353-
match language {
354-
&Language::Rust => "std::mem::transmute",
355-
&Language::C => "cast<double, uint64_t>",
356-
},
357-
value_for_array(64, i),
358-
match language {
359-
&Language::Rust => " as u64",
360-
&Language::C => "",
361-
}
362-
)
363-
})
364-
.collect::<Vec<_>>()
365-
.join(","),
366-
_ => unreachable!("populate random: {:#?}", self),
333+
} => {
334+
let (prefix, cast_prefix, cast_suffix, suffix) = match (language, bit_len) {
335+
(&Language::Rust, 32) => ("[", "f32::from_bits(", ")", "]"),
336+
(&Language::Rust, 64) => ("[", "f64::from_bits(", ")", "]"),
337+
(&Language::C, 32) => ("{", "cast<float, uint32_t>(", ")", "}"),
338+
(&Language::C, 64) => ("{", "cast<double, uint64_t>(", ")", "}"),
339+
_ => unreachable!(),
340+
};
341+
format!(
342+
"{prefix}{body}{suffix}",
343+
body = (0..(simd_len.unwrap_or(1) * vec_len.unwrap_or(1) + loads - 1))
344+
.format_with(", ", |i, fmt| fmt(&format_args!(
345+
"{cast_prefix}{src}{cast_suffix}",
346+
src = value_for_array(*bit_len, i)
347+
)))
348+
)
349+
}
350+
_ => unimplemented!("populate random: {:#?}", self),
367351
}
368352
}
369353

0 commit comments

Comments
 (0)