Feature: new svd-parser based backend (#25)

* Implement new svd-parser based frontend & filtering
* CLI
  * Add parameter `--use-legacy` for old parser
  * Add parameter `--validate` for SVD validation alternatives which is supported by the new frontend

README.md

@@ -20,6 +20,9 @@ cd keelhaul-cli
 cargo run -- --help
 ```
 
+N.b., SoC Hub chip verification so far has used the custom parser available for all subcommands via
+`--use-legacy`.
+
 ## SoC integration guide (register-selftest)
 
 See also. [register-selftest](./register-selftest/README.md).

keelhaul-cli/src/main.rs

@@ -38,6 +38,10 @@ enum Command {
 
         #[arg(long = "validate", default_value = ValidateLevel(keelhaul::ValidateLevel::Disabled), requires = "svd")]
         validate_level: ValidateLevel,
+
+        /// Use the original, custom SVD parser instead of the new `svd-parser` based parser
+        #[arg(long, action = clap::ArgAction::SetTrue, requires = "svd")]
+        use_legacy: bool,
     },
     /// List all top level items (peripherals or subsystems) in the supplied sources
     ///
@@ -53,6 +57,10 @@ enum Command {
         #[arg(long = "validate", default_value = ValidateLevel(keelhaul::ValidateLevel::Disabled), requires = "svd")]
         validate_level: ValidateLevel,
 
+        /// Use the original, custom SVD parser instead of the new `svd-parser` based parser
+        #[arg(long, action = clap::ArgAction::SetTrue, requires = "svd")]
+        use_legacy: bool,
+
         /// Only list peripherals without register counts
         #[arg(long, action = clap::ArgAction::SetTrue)]
         no_count: bool,
@@ -75,6 +83,10 @@ enum Command {
 
         #[arg(long = "validate", default_value = ValidateLevel(keelhaul::ValidateLevel::Disabled), requires = "svd")]
         validate_level: ValidateLevel,
+
+        /// Use the original, custom SVD parser instead of the new `svd-parser` based parser
+        #[arg(long, action = clap::ArgAction::SetTrue, requires = "svd")]
+        use_legacy: bool,
     },
     /// Count the number of readable registers with a known reset value in the inputs
     CountResetValues {
@@ -87,6 +99,10 @@ enum Command {
 
         #[arg(long = "validate", default_value = ValidateLevel(keelhaul::ValidateLevel::Disabled), requires = "svd")]
         validate_level: ValidateLevel,
+
+        /// Use the original, custom SVD parser instead of the new `svd-parser` based parser
+        #[arg(long, action = clap::ArgAction::SetTrue, requires = "svd")]
+        use_legacy: bool,
     },
     /// Generate metadata tests
     #[command(name = "gen-regtest")]
@@ -101,6 +117,10 @@ enum Command {
         #[arg(long = "validate", default_value = ValidateLevel(keelhaul::ValidateLevel::Disabled), requires = "svd")]
         validate_level: ValidateLevel,
 
+        /// Use the original, custom SVD parser instead of the new `svd-parser` based parser
+        #[arg(long, action = clap::ArgAction::SetTrue, requires = "svd")]
+        use_legacy: bool,
+
         /// Type of test to be generated. Chain multiple for more kinds.
         #[arg(short = 't', long = "test", required = true, action = clap::ArgAction::Append)]
         tests_to_generate: Vec<TestKind>,
@@ -434,24 +454,17 @@ fn main() -> anyhow::Result<()> {
         .command
         .as_ref()
         .and_then(|cmd| cmd.arch().map(|arch| arch.into()));
-    if let Some(cmd) = cli.command.as_ref() {
-        if cmd
-            .validate_level()
-            .is_some_and(|l| l.0 != keelhaul::ValidateLevel::Disabled)
-        {
-            anyhow::bail!("validate level not implemented");
-        }
-    }
 
     if let Some(cmd) = &cli.command {
         match cmd {
             Command::DryRun {
                 input: _input,
                 arch: _arch,
                 validate_level: _,
+                use_legacy,
             } => {
                 let sources = sources.unwrap();
-                match keelhaul::dry_run(&sources, arch.unwrap()).with_context(|| {
+                match keelhaul::dry_run(&sources, arch.unwrap(), *use_legacy).with_context(|| {
                     format!("could not execute dry run for arch: {arch:?}, sources: {sources:?}")
                 }) {
                     Ok(_) => println!("keelhaul: dry run completed successfully"),
@@ -465,22 +478,26 @@ fn main() -> anyhow::Result<()> {
                 sorting,
                 validate_level: _,
                 no_rubric,
+                use_legacy,
             } => ls_top(
                 &sources.unwrap(),
                 arch.unwrap(),
                 *sorting,
                 *no_count,
                 *no_rubric,
+                *use_legacy,
             )?,
             Command::CountRegisters {
                 input: _input,
                 validate_level: _,
                 arch: _arch,
+                use_legacy,
             } => {
                 let output = keelhaul::count_registers_svd(
                     &sources.unwrap(),
                     arch.unwrap(),
                     &keelhaul::Filters::all(),
+                    *use_legacy,
                 )?;
                 println!("{output}");
             }
@@ -496,6 +513,7 @@ fn main() -> anyhow::Result<()> {
                 validate_level: _,
                 filter_top_regex,
                 filter_path_regex,
+                use_legacy,
             } => {
                 let mut config = keelhaul::CodegenConfig::default()
                     .tests_to_generate(tests_to_generate.iter().cloned().map(|tk| tk.0).collect())
@@ -527,23 +545,27 @@ fn main() -> anyhow::Result<()> {
                     sources.unwrap(),
                     *no_format,
                     *use_zero_as_default_reset,
+                    *use_legacy,
                 )?
             }
             Command::CountResetValues {
                 input: _input,
                 arch: _arch,
                 validate_level: _,
+                use_legacy,
             } => {
                 let sources = sources.unwrap();
                 let total = keelhaul::count_registers_svd(
                     &sources,
                     arch.unwrap(),
                     &keelhaul::Filters::all(),
+                    *use_legacy,
                 )?;
                 let with_reset = keelhaul::count_readable_registers_with_reset_value(
                     &sources,
                     arch.unwrap(),
                     &keelhaul::Filters::all(),
+                    *use_legacy,
                 )?;
                 let percent = (with_reset as f64 / total as f64) * 100.;
                 println!(
@@ -604,16 +626,25 @@ fn generate(
     sources: Vec<keelhaul::ModelSource>,
     no_format: bool,
     use_zero_as_default_reset: bool,
+    use_legacy: bool,
 ) -> Result<(), anyhow::Error> {
     let output = if no_format {
-        keelhaul::generate_tests(&sources, arch, &config, &filters, use_zero_as_default_reset)?
+        keelhaul::generate_tests(
+            &sources,
+            arch,
+            &config,
+            &filters,
+            use_zero_as_default_reset,
+            use_legacy,
+        )?
     } else {
         keelhaul::generate_tests_with_format(
             &sources,
             arch,
             &config,
             &filters,
             use_zero_as_default_reset,
+            use_legacy,
         )?
     };
     println!("{output}");
@@ -626,8 +657,9 @@ fn ls_top(
     sorting: Sorting,
     no_count: bool,
     no_rubric: bool,
+    use_legacy: bool,
 ) -> Result<(), anyhow::Error> {
-    let mut top_and_count = keelhaul::list_top(sources, arch)?;
+    let mut top_and_count = keelhaul::list_top(sources, arch, use_legacy)?;
     if top_and_count.is_empty() {
         println!("keelhaul: no peripherals found in input");
     }

keelhaul/Cargo.toml

@@ -32,3 +32,4 @@ svd = { package = "svd-rs", version = "0.14.8", features = [
 rustfmt = { version = "0.10.0", optional = true }
 chrono = { version = "0.4.37", default-features = false, features = ["now"] }
 indoc = "2.0.5"
+svd-parser = { version = "0.14.5", features = ["derive-from", "expand"] }

keelhaul/src/api.rs

@@ -89,8 +89,13 @@ impl str::FromStr for TestKind {
 /// Run the parser on the inputs without doing anything
 ///
 /// Good for checking whether the input files can be parsed by Keelhaul.
-pub fn dry_run(sources: &[ModelSource], arch: ArchWidth) -> Result<(), ApiError> {
-    parse_registers(sources, arch, &Filters::all(), false)?;
+///
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
+pub fn dry_run(sources: &[ModelSource], arch: ArchWidth, use_legacy: bool) -> Result<(), ApiError> {
+    parse_registers(sources, arch, &Filters::all(), false, use_legacy)?;
 
     Ok(())
 }
@@ -105,11 +110,14 @@ pub fn dry_run(sources: &[ModelSource], arch: ArchWidth) -> Result<(), ApiError>
 ///
 /// * `use_zero_as_default_reset` - Assume zero as the default reset value if not provided by the
 ///   source file. Provided for convenience, as `0` is a very common reset value.
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 fn parse_registers(
     sources: &[ModelSource],
     arch: ArchWidth,
     filters: &Filters,
     use_zero_as_default_reset: bool,
+    use_legacy: bool,
 ) -> Result<model::Registers, ApiError> {
     for src in sources {
         match src.format {
@@ -129,14 +137,9 @@ fn parse_registers(
     for src in sources {
         match src.format {
             SourceFormat::Svd(vlevel) => {
-                if vlevel != ValidateLevel::Disabled {
-                    return Err(
-                        NotImplementedError::UnsupportedOption(format!("{:?}", vlevel)).into(),
-                    );
-                }
                 let default_reset_value = use_zero_as_default_reset.then_some(0);
 
-                let regs =
+                let regs = if use_legacy {
                     // Safety: max 3 levels of hierarchy (periph + cluster + reg)
                     unsafe {
                         crate::frontend::svd_legacy::parse_svd_into_registers(
@@ -146,8 +149,17 @@ fn parse_registers(
                             default_reset_value,
                         )
                     }
-                    .map_err(Box::new)?;
-                registers.push(regs);
+                    .map_err(Box::new)?
+                } else {
+                    crate::frontend::svd_parser::parse_svd_into_registers(
+                        src.path(),
+                        arch.into(),
+                        filters,
+                        vlevel,
+                    )
+                    .map_err(Box::new)?
+                };
+                registers.push(regs)
             }
             SourceFormat::Ieee1685 => todo!(),
         }
@@ -156,12 +168,17 @@ fn parse_registers(
     Ok(registers.into_iter().next().unwrap())
 }
 
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 fn parse_registers_for_analysis(
     sources: &[ModelSource],
     filters: &Filters,
     arch: ArchWidth,
+    use_legacy: bool,
 ) -> Result<Vec<Box<dyn analysis::AnalyzeRegister>>, ApiError> {
-    Ok(parse_registers(sources, arch, filters, false)?
+    Ok(parse_registers(sources, arch, filters, false, use_legacy)?
         .clone()
         .into_iter()
         .map(|reg| Box::new(reg) as Box<dyn analysis::AnalyzeRegister>)
@@ -195,54 +212,81 @@ fn apply_fmt(input: String) -> String {
     }
 }
 
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 pub fn generate_tests(
     sources: &[ModelSource],
     arch_ptr_size: ArchWidth,
     test_cfg: &CodegenConfig,
     filters: &Filters,
     use_zero_as_default_reset: bool,
+    use_legacy: bool,
 ) -> Result<String, ApiError> {
-    let registers = parse_registers(sources, arch_ptr_size, filters, use_zero_as_default_reset)?;
+    let registers = parse_registers(
+        sources,
+        arch_ptr_size,
+        filters,
+        use_zero_as_default_reset,
+        use_legacy,
+    )?;
     let test_cases = codegen::RegTestCases::from_registers(&registers, test_cfg);
     // FIXME: it would be good to have this message prior to generation
     info!("Wrote {} test cases.", test_cases.test_case_count);
 
     Ok(test_cases.to_tokens().to_string())
 }
 
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 #[cfg(feature = "rustfmt")]
 pub fn generate_tests_with_format(
     sources: &[ModelSource],
     arch_ptr_size: ArchWidth,
     test_cfg: &codegen::CodegenConfig,
     filters: &Filters,
     use_zero_as_default_reset: bool,
+    use_legacy: bool,
 ) -> Result<String, ApiError> {
     let s = generate_tests(
         sources,
         arch_ptr_size,
         test_cfg,
         filters,
         use_zero_as_default_reset,
+        use_legacy,
     )?;
     Ok(apply_fmt(s))
 }
 
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 pub fn count_registers_svd(
     sources: &[ModelSource],
     arch: ArchWidth,
     filters: &Filters,
+    use_legacy: bool,
 ) -> Result<usize, ApiError> {
-    let registers = parse_registers_for_analysis(sources, filters, arch)?;
+    let registers = parse_registers_for_analysis(sources, filters, arch, use_legacy)?;
     Ok(registers.len())
 }
 
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 pub fn count_readable_registers_with_reset_value(
     sources: &[ModelSource],
     arch: ArchWidth,
     filters: &Filters,
+    use_legacy: bool,
 ) -> Result<usize, ApiError> {
-    let registers = parse_registers_for_analysis(sources, filters, arch)?;
+    let registers = parse_registers_for_analysis(sources, filters, arch, use_legacy)?;
     Ok(registers
         .iter()
         .filter(|reg| reg.is_readable() && reg.has_reset_value())
@@ -252,11 +296,17 @@ pub fn count_readable_registers_with_reset_value(
 /// Returns top level containers (peripherals or subsystems) and the number of registers in each
 ///
 /// `Vec<(container, register count)>`
+///
+/// # Arguments
+///
+/// * `use_legacy` - Use the original, custom SVD parser instead of the new `svd-parser` based
+///   parser
 pub fn list_top(
     sources: &[ModelSource],
     arch: ArchWidth,
+    use_legacy: bool,
 ) -> Result<Vec<(String, usize)>, ApiError> {
-    let registers = parse_registers_for_analysis(sources, &Filters::all(), arch)?;
+    let registers = parse_registers_for_analysis(sources, &Filters::all(), arch, use_legacy)?;
 
     let tops = registers
         .iter()

keelhaul/src/frontend/mod.rs

@@ -1,3 +1,4 @@
 //! The point of any frontend is to convert the given source file into a model of [Registers]
 
 pub(crate) mod svd_legacy;
+pub(crate) mod svd_parser;