Sharpening the parsing logic:

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)

Author: madhav-madhusoodanan

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)",
             _ => "",
         }
     }

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 { "" },
+        )
     }
 }

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,

crates/intrinsic-test/src/x86/types.rs

@@ -11,15 +11,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())
     }
@@ -59,23 +53,30 @@ impl IntrinsicTypeDefinition for X86IntrinsicType {
         // functionality below this line.
         // Currently all the intrinsics that have an "_"
         // is ignored.
-        if let Some(_) = s.matches("_").next() {
+        if s.matches("_").next().is_some() {
             return Err(String::from("This functionality needs to be implemented"));
         };
 
         // TODO: make the unwrapping safe
         let kind = TypeKind::from_str(s_split.unwrap()).expect("Unable to parse type!");
-        let mut ptr_constant = false;
+
+        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 mut constant = false;
         let mut ptr = false;
 
-        if let Some(_) = s.matches("const*").next() {
-            ptr_constant = true;
-        };
-        if let Some(_) = s.matches("const").next() {
+        if s.matches("const").next().is_some() {
             constant = true;
         };
-        if let Some(_) = s.matches("*").next() {
+        if s.matches("*").next().is_some() {
             ptr = true;
         };
 

crates/intrinsic-test/src/x86/xml_parser.rs

@@ -1,6 +1,6 @@
 use crate::common::argument::{Argument, ArgumentList};
 use crate::common::intrinsic::Intrinsic;
-use crate::common::intrinsic_helpers::{IntrinsicType, IntrinsicTypeDefinition};
+use crate::common::intrinsic_helpers::{IntrinsicType, IntrinsicTypeDefinition, TypeKind};
 
 use serde::{Deserialize, Deserializer};
 use std::path::Path;
@@ -38,7 +38,7 @@ struct Data {
 #[derive(Deserialize)]
 struct XMLIntrinsic {
     #[serde(rename = "return")]
-    return_data: Return,
+    return_data: Parameter,
     #[serde(rename = "@name")]
     name: String,
     #[serde(rename = "@tech")]
@@ -65,12 +65,6 @@ struct Parameter {
     var_name: String,
 }
 
-#[derive(Deserialize)]
-struct Return {
-    #[serde(rename = "@type", default)]
-    type_data: String,
-}
-
 #[derive(Deserialize, Debug)]
 struct Instruction {
     #[serde(rename = "@name")]
@@ -86,7 +80,6 @@ pub fn get_xml_intrinsics(
     let data: Data =
         quick_xml::de::from_reader(reader).expect("failed to deserialize the source XML file");
 
-    // println!("{} intrinsics found", data.intrinsics.len());
     let parsed_intrinsics: Vec<Intrinsic<X86IntrinsicType>> = data
         .intrinsics
         .into_iter()
@@ -99,26 +92,43 @@ pub fn get_xml_intrinsics(
     Ok(parsed_intrinsics)
 }
 
+fn parse_observable(param: &Parameter, target: &String) -> Result<X86IntrinsicType, String> {
+    let ty = X86IntrinsicType::from_c(param.type_data.as_str(), target);
+
+    if let Err(_) = ty {
+        return ty;
+    }
+    let mut ty_bit_len = param.etype.clone();
+    ty_bit_len.retain(|c| c.is_numeric());
+    let ty_bit_len = str::parse::<u32>(ty_bit_len.as_str()).ok();
+    let mut ty = ty.unwrap();
+    let ty_bit_len = match ty_bit_len {
+        None => match ty.kind {
+            TypeKind::Int(_) => Some(32),
+            TypeKind::Float => Some(32),
+            TypeKind::BFloat => Some(16),
+            TypeKind::Void => Some(param.memwidth as u32),
+            _ => None,
+        },
+        ty => ty,
+    };
+    ty.set_bit_len(ty_bit_len);
+    Ok(ty)
+}
+
 fn xml_to_intrinsic(
     intr: XMLIntrinsic,
     target: &String,
 ) -> Result<Intrinsic<X86IntrinsicType>, Box<dyn std::error::Error>> {
     let name = intr.name;
-    let results = X86IntrinsicType::from_c(&intr.return_data.type_data, target)?;
-
+    let result = parse_observable(&intr.return_data, target);
     let args_check = intr.parameters.into_iter().enumerate().map(|(i, param)| {
-        let constraint = None;
-        let ty = X86IntrinsicType::from_c(param.type_data.as_str(), target);
-
+        let ty = parse_observable(&param, target);
         if let Err(_) = ty {
             return None;
         }
-        let mut ty_bit_len = param.etype.clone();
-        ty_bit_len.retain(|c| c.is_numeric());
-        let ty_bit_len = str::parse::<u32>(ty_bit_len.as_str()).ok();
-        let mut ty = ty.unwrap();
-        ty.set_bit_len(ty_bit_len);
-        let mut arg = Argument::<X86IntrinsicType>::new(i, param.var_name, ty, constraint);
+        let constraint = None;
+        let mut arg = Argument::<X86IntrinsicType>::new(i, param.var_name, ty.unwrap(), constraint);
         let IntrinsicType {
             ref mut constant, ..
         } = arg.ty.0;
@@ -132,13 +142,20 @@ fn xml_to_intrinsic(
     if args.iter().any(|elem| elem.is_none()) {
         return Err(Box::from("intrinsic isn't fully supported in this test!"));
     }
-    let args = args.into_iter().map(|e| e.unwrap()).collect::<Vec<_>>();
+    let args = args
+        .into_iter()
+        .map(|e| e.unwrap())
+        .filter(|arg| arg.ty.ptr || arg.ty.kind != TypeKind::Void)
+        .collect::<Vec<_>>();
     let arguments = ArgumentList::<X86IntrinsicType> { args };
 
+    if let Err(message) = result {
+        return Err(Box::from(message));
+    }
     Ok(Intrinsic {
         name,
         arguments,
-        results: results,
+        results: result.unwrap(),
         arch_tags: intr.cpuid,
     })
 }