Format f16 values normally, rather than use the bit patterns

sayantn · sayantn · commit d90008157ba1 · 2025-11-30T02:16:42.000+05:30
diff --git a/crates/intrinsic-test/src/arm/config.rs b/crates/intrinsic-test/src/arm/config.rs
@@ -38,11 +38,7 @@ std::ostream& operator<<(std::ostream& os, poly128_t value) {
 #endif
 
 std::ostream& operator<<(std::ostream& os, float16_t value) {
-    uint16_t temp = 0;
-    memcpy(&temp, &value, sizeof(float16_t));
-    std::stringstream ss;
-    ss << "0x" << std::setfill('0') << std::setw(4) << std::hex << temp;
-    os << ss.str();
+    os << static_cast<float>(value);
     return os;
 }
 
@@ -52,92 +48,7 @@ std::ostream& operator<<(std::ostream& os, uint8_t value) {
 }
 "#;
 
-// Format f16 values (and vectors containing them) in a way that is consistent with C.
-pub const PLATFORM_RUST_DEFINITIONS: &str = r#"
-/// Used to continue `Debug`ging SIMD types as `MySimd(1, 2, 3, 4)`, as they
-/// were before moving to array-based simd.
-#[inline]
-fn debug_simd_finish<T: core::fmt::Debug, const N: usize>(
-    formatter: &mut core::fmt::Formatter<'_>,
-    type_name: &str,
-    array: &[T; N],
-) -> core::fmt::Result {
-    core::fmt::Formatter::debug_tuple_fields_finish(
-        formatter,
-        type_name,
-        &core::array::from_fn::<&dyn core::fmt::Debug, N, _>(|i| &array[i]),
-    )
-}
-
-#[repr(transparent)]
-struct Hex<T>(T);
-
-impl<T: DebugHexF16> core::fmt::Debug for Hex<T> {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        <T as DebugHexF16>::fmt(&self.0, f)
-    }
-}
-
-fn debug_f16<T: DebugHexF16>(x: T) -> impl core::fmt::Debug {
-    Hex(x)
-}
-
-trait DebugHexF16 {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result;
-}
-
-impl DebugHexF16 for f16 {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        write!(f, "{:#06x?}", self.to_bits())
-    }
-}
-
-impl DebugHexF16 for float16x4_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 4]>(*self) };
-        debug_simd_finish(f, "float16x4_t", &array)
-    }
-}
-
-impl DebugHexF16 for float16x8_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 8]>(*self) };
-        debug_simd_finish(f, "float16x8_t", &array)
-    }
-}
-
-impl DebugHexF16 for float16x4x2_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x4x2_t", &[Hex(self.0), Hex(self.1)])
-    }
-}
-impl DebugHexF16 for float16x4x3_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x4x3_t", &[Hex(self.0), Hex(self.1), Hex(self.2)])
-    }
-}
-impl DebugHexF16 for float16x4x4_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x4x4_t", &[Hex(self.0), Hex(self.1), Hex(self.2), Hex(self.3)])
-    }
-}
-
-impl DebugHexF16 for float16x8x2_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x8x2_t", &[Hex(self.0), Hex(self.1)])
-    }
-}
-impl DebugHexF16 for float16x8x3_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x8x3_t", &[Hex(self.0), Hex(self.1), Hex(self.2)])
-    }
-}
-impl DebugHexF16 for float16x8x4_t {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        debug_simd_finish(f, "float16x8x4_t", &[Hex(self.0), Hex(self.1), Hex(self.2), Hex(self.3)])
-    }
-}
- "#;
+pub const PLATFORM_RUST_DEFINITIONS: &str = "";
 
 pub const PLATFORM_RUST_CFGS: &str = r#"
 #![cfg_attr(target_arch = "arm", feature(stdarch_arm_neon_intrinsics))]
diff --git a/crates/intrinsic-test/src/common/intrinsic_helpers.rs b/crates/intrinsic-test/src/common/intrinsic_helpers.rs
@@ -368,19 +368,8 @@ pub trait IntrinsicTypeDefinition: Deref<Target = IntrinsicType> {
     /// Generates a std::cout for the intrinsics results that will match the
     /// rust debug output format for the return type. The generated line assumes
     /// there is an int i in scope which is the current pass number.
-    ///
-    /// The `intrinsic-test` crate compares the output of C and Rust intrinsics. Currently, It uses
-    /// a string representation of the output value to compare. In C, f16 values are currently printed
-    /// as hexadecimal integers. Since https://github.com/rust-lang/rust/pull/127013, rust does print
-    /// them as decimal floating point values. To keep the intrinsics tests working, for now, format
-    /// vectors containing f16 values like C prints them.
     fn print_result_rust(&self) -> String {
-        let return_value = match self.kind() {
-            TypeKind::Float if self.inner_size() == 16 => "debug_f16(__return_value)",
-            _ => "format_args!(\"{__return_value:.150?}\")",
-        };
-
-        String::from(return_value)
+        String::from("format_args!(\"{__return_value:.150?}\")")
     }
 
     /// To enable architecture-specific logic
diff --git a/crates/intrinsic-test/src/x86/config.rs b/crates/intrinsic-test/src/x86/config.rs
@@ -142,71 +142,6 @@ fn debug_simd_finish<T: core::fmt::Debug, const N: usize>(
     )
 }
 
-#[repr(transparent)]
-struct Hex<T>(T);
-
-impl<T: DebugHexF16> core::fmt::Debug for Hex<T> {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        <T as DebugHexF16>::fmt(&self.0, f)
-    }
-}
-
-fn debug_f16<T: DebugHexF16>(x: T) -> impl core::fmt::Debug {
-    Hex(x)
-}
-
-trait DebugHexF16 {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result;
-}
-
-impl DebugHexF16 for f16 {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        write!(f, "{:#06x?}", self.to_bits())
-    }
-}
-
-impl DebugHexF16 for __m128h {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 8]>(*self) };
-        debug_simd_finish(f, "__m128h", &array)
-    }
-}
-
-impl DebugHexF16 for __m128i {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 8]>(*self) };
-        debug_simd_finish(f, "__m128i", &array)
-    }
-}
-
-impl DebugHexF16 for __m256h {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 16]>(*self) };
-        debug_simd_finish(f, "__m256h", &array)
-    }
-}
-
-impl DebugHexF16 for __m256i {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 16]>(*self) };
-        debug_simd_finish(f, "__m256i", &array)
-    }
-}
-
-impl DebugHexF16 for __m512h {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 32]>(*self) };
-        debug_simd_finish(f, "__m512h", &array)
-    }
-}
-
-impl DebugHexF16 for __m512i {
-    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
-        let array = unsafe { core::mem::transmute::<_, [Hex<f16>; 32]>(*self) };
-        debug_simd_finish(f, "__m512i", &array)
-    }
-}
-
 trait DebugAs<T> {
     fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result;
 }
@@ -232,7 +167,7 @@ macro_rules! impl_debug_as {
     };
 }
 
-impl_debug_as!(__m128i, "__m128i", 128, [u8, i8, u16, i16, u32, i32, u64, i64]);
+impl_debug_as!(__m128i, "__m128i", 128, [u8, i8, u16, i16, u32, i32, u64, i64, f16]);
 impl_debug_as!(__m256i, "__m256i", 256, [u8, i8, u16, i16, u32, i32, u64, i64]);
 impl_debug_as!(__m512i, "__m512i", 512, [u8, i8, u16, i16, u32, i32, u64, i64]);
 impl_debug_as!(__m128h, "__m128h", 128, [f32]);
@@ -336,11 +271,7 @@ pub const PLATFORM_C_FORWARD_DECLARATIONS: &str = r#"
 pub const PLATFORM_C_DEFINITIONS: &str = r#"
 
 std::ostream& operator<<(std::ostream& os, _Float16 value) {
-    uint16_t temp = 0;
-    memcpy(&temp, &value, sizeof(_Float16));
-    std::stringstream ss;
-    ss << "0x" << std::setfill('0') << std::setw(4) << std::hex << temp;
-    os << ss.str();
+    os << static_cast<float>(value);
     return os;
 }
 
diff --git a/crates/intrinsic-test/src/x86/types.rs b/crates/intrinsic-test/src/x86/types.rs
@@ -172,29 +172,7 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
     /// rust debug output format for the return type. The generated line assumes
     /// there is an int i in scope which is the current pass number.
     fn print_result_c(&self, indentation: Indentation, additional: &str) -> String {
-        let lanes = if self.num_vectors() > 1 {
-            (0..self.num_vectors())
-                .map(|vector| {
-                    format!(
-                        r#""{ty}(" << {lanes} << ")""#,
-                        ty = self.c_single_vector_type(),
-                        lanes = (0..self.num_lanes())
-                            .map(move |idx| -> std::string::String {
-                                format!(
-                                    "{cast}{lane_fn}(__return_value.val[{vector}], {lane})",
-                                    cast = self.generate_final_type_cast(),
-                                    lane_fn = self.get_lane_function(),
-                                    lane = idx,
-                                    vector = vector,
-                                )
-                            })
-                            .collect::<Vec<_>>()
-                            .join(r#" << ", " << "#)
-                    )
-                })
-                .collect::<Vec<_>>()
-                .join(r#" << ", " << "#)
-        } else if self.num_lanes() > 1 {
+        let lanes = if self.num_lanes() > 1 {
             (0..self.num_lanes())
                 .map(|idx| -> std::string::String {
                     let cast_type = self.c_promotion();
@@ -283,12 +261,9 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
 
     fn print_result_rust(&self) -> String {
         let return_value = match self.kind() {
-            TypeKind::Float if self.inner_size() == 16 => "debug_f16(__return_value)".to_string(),
-            TypeKind::Float
-                if self.inner_size() == 32
-                    && ["__m512h"].contains(&self.param.type_data.as_str()) =>
-            {
-                "debug_as::<_, f32>(__return_value)".to_string()
+            // `_mm{256}_cvtps_ph` has return type __m128i but contains f16 values
+            TypeKind::Float if self.param.type_data == "__m128i" => {
+                "format_args!(\"{:.150?}\", debug_as::<_, f16>(__return_value))".to_string()
             }
             TypeKind::Int(_)
                 if ["__m128i", "__m256i", "__m512i"].contains(&self.param.type_data.as_str()) =>
@@ -463,6 +438,17 @@ impl X86IntrinsicType {
                     }
                 }
 
+                // a few intrinsics have wrong `etype` field in the XML
+                // - _mm512_reduce_add_ph
+                // - _mm512_reduce_mul_ph
+                // - _mm512_reduce_min_ph
+                // - _mm512_reduce_max_ph
+                // - _mm512_conj_pch
+                if param.type_data == "__m512h" && param.etype == "FP32" {
+                    data.bit_len = Some(16);
+                    data.simd_len = Some(32);
+                }
+
                 let mut result = X86IntrinsicType {
                     data,
                     param: param.clone(),
