Sharpening the parsing logic:

madhav-madhusoodanan · madhav-madhusoodanan · commit 287b83c73bc1 · 2025-06-20T10:56:14.000+05:30
1. filtered any intrinsic arguments where the only parameter is of type
"void"
2. Better parsing for intrinsic return type
3. Filtering intrinsics that are difficult to test (void returns)
4. Reduced the variants in TypeKind (which differed only in bit_len)
diff --git a/crates/intrinsic-test/src/common/intrinsic_helpers.rs b/crates/intrinsic-test/src/common/intrinsic_helpers.rs
@@ -12,12 +12,10 @@ use super::values::value_for_array;
 pub enum TypeKind {
     BFloat,
     Float,
-    Double,
 
     // if signed, then the inner value is true
     Int(bool),
     Char(bool),
-    Short(bool),
     Poly,
     Void,
 }
@@ -29,9 +27,8 @@ impl FromStr for TypeKind {
         match s {
             "bfloat" => Ok(Self::BFloat),
             "float" => Ok(Self::Float),
-            "double" => Ok(Self::Double),
-            "int" | "long" => Ok(Self::Int(true)),
-            "short" => Ok(Self::Short(true)),
+            "double" => Ok(Self::Float),
+            "int" | "long" | "short" => Ok(Self::Int(true)),
             "poly" => Ok(Self::Poly),
             "char" => Ok(Self::Char(true)),
             "uint" | "unsigned" => Ok(Self::Int(false)),
@@ -49,15 +46,12 @@ impl fmt::Display for TypeKind {
             match self {
                 Self::BFloat => "bfloat",
                 Self::Float => "float",
-                Self::Double => "double",
                 Self::Int(true) => "int",
                 Self::Int(false) => "uint",
                 Self::Poly => "poly",
                 Self::Void => "void",
                 Self::Char(true) => "char",
                 Self::Char(false) => "unsigned char",
-                Self::Short(true) => "short",
-                Self::Short(false) => "unsigned short",
             }
         )
     }
@@ -72,7 +66,6 @@ impl TypeKind {
             Self::Int(false) => "uint",
             Self::Poly => "poly",
             Self::Char(true) => "char",
-            Self::Double => "double",
             _ => unreachable!("Not used: {:#?}", self),
         }
     }
@@ -126,7 +119,7 @@ impl IntrinsicType {
         if let Some(bl) = self.bit_len {
             bl
         } else {
-            unreachable!("")
+            unreachable!("{}", self.kind)
         }
     }
 
@@ -159,11 +152,17 @@ impl IntrinsicType {
     }
 
     pub fn c_scalar_type(&self) -> String {
-        format!(
-            "{prefix}{bits}_t",
-            prefix = self.kind().c_prefix(),
-            bits = self.inner_size()
-        )
+        match self {
+            IntrinsicType {
+                kind: TypeKind::Char(_),
+                ..
+            } => String::from("char"),
+            _ => format!(
+                "{prefix}{bits}_t",
+                prefix = self.kind().c_prefix(),
+                bits = self.inner_size()
+            ),
+        }
     }
 
     pub fn rust_scalar_type(&self) -> String {
@@ -198,6 +197,21 @@ impl IntrinsicType {
                 128 => "",
                 _ => panic!("invalid bit_len"),
             },
+            IntrinsicType {
+                kind: TypeKind::Float,
+                bit_len: Some(bit_len),
+                ..
+            } => match bit_len {
+                16 => "(float16_t)",
+                32 => "(float)",
+                64 => "(double)",
+                128 => "",
+                _ => panic!("invalid bit_len"),
+            },
+            IntrinsicType {
+                kind: TypeKind::Char(_),
+                ..
+            } => "(char)",
             _ => "",
         }
     }
diff --git a/crates/intrinsic-test/src/x86/intrinsic.rs b/crates/intrinsic-test/src/x86/intrinsic.rs
@@ -1,7 +1,7 @@
 use crate::common::argument::ArgumentList;
 use crate::common::indentation::Indentation;
 use crate::common::intrinsic::{Intrinsic, IntrinsicDefinition};
-use crate::common::intrinsic_helpers::IntrinsicType;
+use crate::common::intrinsic_helpers::{IntrinsicType, IntrinsicTypeDefinition, TypeKind};
 use std::ops::{Deref, DerefMut};
 
 #[derive(Debug, Clone, PartialEq)]
@@ -38,7 +38,69 @@ impl IntrinsicDefinition<X86IntrinsicType> for Intrinsic<X86IntrinsicType> {
     /// 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.
-    fn print_result_c(&self, _indentation: Indentation, _additional: &str) -> String {
-        todo!("print_result_c in Intrinsic<X86IntrinsicType> needs to be implemented!");
+    fn print_result_c(&self, indentation: Indentation, additional: &str) -> String {
+        let lanes = if self.results().num_vectors() > 1 {
+            (0..self.results().num_vectors())
+                .map(|vector| {
+                    format!(
+                        r#""{ty}(" << {lanes} << ")""#,
+                        ty = self.results().c_single_vector_type(),
+                        lanes = (0..self.results().num_lanes())
+                            .map(move |idx| -> std::string::String {
+                                format!(
+                                    "{cast}{lane_fn}(__return_value.val[{vector}], {lane})",
+                                    cast = self.results().c_promotion(),
+                                    lane_fn = self.results().get_lane_function(),
+                                    lane = idx,
+                                    vector = vector,
+                                )
+                            })
+                            .collect::<Vec<_>>()
+                            .join(r#" << ", " << "#)
+                    )
+                })
+                .collect::<Vec<_>>()
+                .join(r#" << ", " << "#)
+        } else if self.results().num_lanes() > 1 {
+            (0..self.results().num_lanes())
+                .map(|idx| -> std::string::String {
+                    format!(
+                        "{cast}{lane_fn}(__return_value, {lane})",
+                        cast = self.results().c_promotion(),
+                        lane_fn = self.results().get_lane_function(),
+                        lane = idx
+                    )
+                })
+                .collect::<Vec<_>>()
+                .join(r#" << ", " << "#)
+        } else {
+            format!(
+                "{promote}cast<{cast}>(__return_value)",
+                cast = match self.results.kind() {
+                    TypeKind::Void => "void".to_string(),
+                    TypeKind::Float if self.results().inner_size() == 64 => "double".to_string(),
+                    TypeKind::Float if self.results().inner_size() == 32 => "float".to_string(),
+                    // TypeKind::Float if self.results().inner_size() == 16 => "float16_t".to_string(),
+                    // TypeKind::Int(true) if self.results().inner_size() == 64 => "long".to_string(),
+                    // TypeKind::Int(false) if self.results().inner_size() == 64 => "unsigned long".to_string(),
+                    // TypeKind::Int(true) if self.results().inner_size() == 32 => "int".to_string(),
+                    // TypeKind::Int(false) if self.results().inner_size() == 32 => "unsigned int".to_string(),
+                    // TypeKind::Int(true) if self.results().inner_size() == 16 => "short".to_string(),
+                    // TypeKind::Int(false) if self.results().inner_size() == 16 => "unsigned short".to_string(),
+                    _ => self.results.c_scalar_type(),
+                },
+                promote = self.results().c_promotion(),
+            )
+        };
+
+        format!(
+            r#"{indentation}std::cout << "Result {additional}-" << i+1 << ": {ty}" << std::fixed << std::setprecision(150) <<  {lanes} << "{close}" << std::endl;"#,
+            ty = if self.results().is_simd() {
+                format!("{}(", self.results().c_type())
+            } else {
+                String::from("")
+            },
+            close = if self.results.is_simd() { ")" } else { "" },
+        )
     }
 }
diff --git a/crates/intrinsic-test/src/x86/mod.rs b/crates/intrinsic-test/src/x86/mod.rs
@@ -6,6 +6,7 @@ mod xml_parser;
 use crate::common::SupportedArchitectureTest;
 use crate::common::cli::ProcessedCli;
 use crate::common::intrinsic::{Intrinsic, IntrinsicDefinition};
+use crate::common::intrinsic_helpers::TypeKind;
 use crate::common::write_file::{write_c_testfiles, write_rust_testfiles};
 use config::build_notices;
 use intrinsic::X86IntrinsicType;
@@ -18,9 +19,24 @@ pub struct X86ArchitectureTest {
 
 impl SupportedArchitectureTest for X86ArchitectureTest {
     fn create(cli_options: ProcessedCli) -> Box<Self> {
-        let intrinsics = get_xml_intrinsics(&cli_options.filename, &cli_options.target)
+        let mut intrinsics = get_xml_intrinsics(&cli_options.filename, &cli_options.target)
             .expect("Error parsing input file");
 
+        intrinsics.sort_by(|a, b| a.name.cmp(&b.name));
+        let intrinsics = intrinsics
+            .into_iter()
+            // Not sure how we would compare intrinsic that returns void.
+            .filter(|i| i.results.kind() != TypeKind::Void)
+            .filter(|i| i.results.kind() != TypeKind::BFloat)
+            .filter(|i| i.arguments().args.len() > 0)
+            .filter(|i| !i.arguments.iter().any(|a| a.ty.kind() == TypeKind::BFloat))
+            // Skip pointers for now, we would probably need to look at the return
+            // type to work out how many elements we need to point to.
+            .filter(|i| !i.arguments.iter().any(|a| a.is_ptr()))
+            .filter(|i| !i.arguments.iter().any(|a| a.ty.inner_size() == 128))
+            .filter(|i| !cli_options.skip.contains(&i.name))
+            .collect::<Vec<_>>();
+
         Box::new(Self {
             intrinsics: intrinsics,
             cli_options: cli_options,
diff --git a/crates/intrinsic-test/src/x86/types.rs b/crates/intrinsic-test/src/x86/types.rs
@@ -1,5 +1,7 @@
 use std::str::FromStr;
 
+use itertools::Itertools;
+
 use super::intrinsic::X86IntrinsicType;
 use crate::common::cli::Language;
 use crate::common::intrinsic_helpers::{IntrinsicType, IntrinsicTypeDefinition, TypeKind};
@@ -11,15 +13,9 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
         let part_1 = match self.kind {
             TypeKind::Int(false) => "unsigned int",
             TypeKind::Char(false) => "unsigned char",
-            TypeKind::Short(false) => "unsigned short",
-            TypeKind::Short(true) => "short",
             _ => self.kind.c_prefix(),
         };
-        let part_2 = if self.ptr {
-            if self.ptr_constant { "* const" } else { "*" }
-        } else {
-            ""
-        };
+        let part_2 = if self.ptr { "*" } else { "" };
 
         String::from(vec![part_0, part_1, part_2].join(" ").trim())
     }
@@ -46,6 +42,7 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
         let mut s_copy = s.to_string();
         s_copy = s_copy
             .replace("*", "")
+            .replace("_", "")
             .replace("constexpr", "")
             .replace("const", "")
             .replace("literal", "");
@@ -55,30 +52,25 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
             .filter_map(|s| if s.len() == 0 { None } else { Some(s) })
             .last();
 
-        // TODO: add more intrinsics by modifying
-        // functionality below this line.
-        // Currently all the intrinsics that have an "_"
-        // is ignored.
-        if let Some(_) = s.matches("_").next() {
-            return Err(String::from("This functionality needs to be implemented"));
-        };
+        let s_split = s_split.map(|s| s.chars().filter(|c| !c.is_numeric()).join(""));
 
         // TODO: make the unwrapping safe
-        let kind = TypeKind::from_str(s_split.unwrap()).expect("Unable to parse type!");
-        let mut ptr_constant = false;
-        let mut constant = false;
-        let mut ptr = false;
+        let kind = TypeKind::from_str(s_split.unwrap().trim()).unwrap_or(TypeKind::Void);
 
-        if let Some(_) = s.matches("const*").next() {
-            ptr_constant = true;
-        };
-        if let Some(_) = s.matches("const").next() {
-            constant = true;
-        };
-        if let Some(_) = s.matches("*").next() {
-            ptr = true;
+        let kind = if s.find("unsigned").is_some() {
+            match kind {
+                TypeKind::Int(_) => TypeKind::Int(false),
+                TypeKind::Char(_) => TypeKind::Char(false),
+                a => a,
+            }
+        } else {
+            kind
         };
 
+        let ptr_constant = false;
+        let constant = s.matches("const").next().is_some();
+        let ptr = s.matches("*").next().is_some();
+
         Ok(X86IntrinsicType(IntrinsicType {
             ptr,
             ptr_constant,
@@ -91,3 +83,67 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
         }))
     }
 }
+
+impl X86IntrinsicType {
+    pub fn from_param(param: &Parameter) -> Result<Self, String> {
+        match Self::from_c(param.type_data.as_str()) {
+            Err(message) => Err(message),
+            Ok(mut ret) => {
+                // First correct the type of the parameter using param.etype.
+                // The assumption is that the parameter of type void may have param.type
+                // as "__m128i", "__mmask8" and the like.
+                ret.set_metadata("etype".to_string(), param.etype.clone());
+                if !param.etype.is_empty() {
+                    match TypeKind::from_str(param.etype.as_str()) {
+                        Ok(value) => {
+                            ret.kind = value;
+                        }
+                        Err(_) => {}
+                    };
+                }
+
+                // check for param.etype.
+                // extract the numeric part and set as bit-len
+                // If param.etype is not present, guess the default bit-len
+
+                let mut etype_processed = param.etype.clone();
+                etype_processed.retain(|c| c.is_numeric());
+
+                match str::parse::<u32>(etype_processed.as_str()) {
+                    Ok(value) => ret.bit_len = Some(value),
+                    Err(_) => {
+                        ret.bit_len = match ret.kind() {
+                            TypeKind::Char(_) => Some(8),
+                            TypeKind::BFloat => Some(16),
+                            TypeKind::Int(_) => Some(32),
+                            TypeKind::Float => Some(32),
+                            _ => None,
+                        };
+                    }
+                }
+
+                // then check the param.type and extract numeric part if there are double
+                // underscores. divide this number with bit-len and set this as simd-len.
+
+                let mut type_processed = param.etype.clone();
+                type_processed.retain(|c| c.is_numeric());
+
+                ret.vec_len = match str::parse::<u32>(etype_processed.as_str()) {
+                    // If bit_len is None, vec_len will be None.
+                    // Else vec_len will be (num_bits / bit_len).
+                    Ok(num_bits) => ret.bit_len.and(Some(num_bits / ret.bit_len.unwrap())),
+                    Err(_) => None,
+                };
+
+                // if param.etype == IMM, then it is a constant.
+                // else it stays unchanged.
+                ret.constant |= param.etype == "IMM";
+
+                Ok(ret)
+            }
+        }
+        // Tile types won't currently reach here, since the intrinsic that involve them
+        // often return "null" type. Such intrinsics are not tested in `intrinsic-test`
+        // currently and are filtered out at `mod.rs`.
+    }
+}
diff --git a/crates/intrinsic-test/src/x86/xml_parser.rs b/crates/intrinsic-test/src/x86/xml_parser.rs
