From deb379e7402340146fd4c6a0fc4dcb4b9e38eb1a Mon Sep 17 00:00:00 2001 From: Seth Pellegrino Date: Thu, 18 May 2023 21:43:34 -0700 Subject: [PATCH 1/4] feat: configurable link section attribute for irqs This change introduces a new config field that allows `svd2rust` to target which linker sections get assigned to the `__INTERRUPTS` static, with reasonable defaults. Previously on RISC-V, the choice was always left up to the compiler, and it seemed to always pick `.rodata`. Unfortunately, in my context, that meant placing the LUT in a memory range that had a lot of highly variable latency, which cost not just time but predictability in servicing interrupts. With this change in place, I'm able to target a particular section (e.g. `.data`, or `.trap.rodata`) for the placement of the static, which grants more granular control over the ultimate loaded memory address. For the full details about the problem, please see: https://github.com/esp-rs/esp-hal/pull/534/commits/e29f3d547dc210e1b73313be6053a2122239a467 --- src/generate/interrupt.rs | 38 ++++++++++++++++++++++++++++++++++++-- src/util.rs | 3 +++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/generate/interrupt.rs b/src/generate/interrupt.rs index 0f826f6a..6ea1785f 100644 --- a/src/generate/interrupt.rs +++ b/src/generate/interrupt.rs @@ -118,6 +118,14 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);")?; } + let link_section_name = config + .interrupt_link_section + .as_deref() + .unwrap_or(".vector_table.interrupts"); + let link_section_attr = quote! { + #[link_section = #link_section_name] + }; + root.extend(quote! { #[cfg(feature = "rt")] extern "C" { @@ -132,7 +140,7 @@ pub fn render( #[cfg(feature = "rt")] #[doc(hidden)] - #[link_section = ".vector_table.interrupts"] + #link_section_attr #[no_mangle] pub static __INTERRUPTS: [Vector; #n] = [ #elements @@ -144,6 +152,14 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);").unwrap(); } + let link_section_name = config + .interrupt_link_section + .as_deref() + .unwrap_or(".vector_table.interrupts"); + let link_section_attr = quote! { + #[link_section = #link_section_name] + }; + root.extend(quote! { #[cfg(feature = "rt")] extern "msp430-interrupt" { @@ -158,7 +174,7 @@ pub fn render( #[cfg(feature = "rt")] #[doc(hidden)] - #[link_section = ".vector_table.interrupts"] + #link_section_attr #[no_mangle] #[used] pub static __INTERRUPTS: @@ -172,6 +188,14 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);")?; } + let link_section_attr = if let Some(section) = &config.interrupt_link_section { + quote! { + #[link_section = #section] + } + } else { + quote! {} + }; + root.extend(quote! { #[cfg(feature = "rt")] extern "C" { @@ -186,6 +210,7 @@ pub fn render( #[cfg(feature = "rt")] #[doc(hidden)] + #link_section_attr #[no_mangle] pub static __EXTERNAL_INTERRUPTS: [Vector; #n] = [ #elements @@ -197,6 +222,14 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);")?; } + let link_section_attr = if let Some(section) = &config.interrupt_link_section { + quote! { + #[link_section = #section] + } + } else { + quote! {} + }; + root.extend(quote! { #[cfg(feature = "rt")] extern "C" { @@ -210,6 +243,7 @@ pub fn render( } #[cfg(feature = "rt")] + #link_section_attr #[doc(hidden)] pub static __INTERRUPTS: [Vector; #n] = [ #elements diff --git a/src/util.rs b/src/util.rs index 1e295a80..6d06bd2c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -59,6 +59,8 @@ pub struct Config { pub source_type: SourceType, #[cfg_attr(feature = "serde", serde(default))] pub log_level: Option, + #[cfg_attr(feature = "serde", serde(default))] + pub interrupt_link_section: Option, } #[derive(Clone, Debug, PartialEq, Eq)] @@ -120,6 +122,7 @@ impl Default for Config { input: None, source_type: SourceType::default(), log_level: None, + interrupt_link_section: None, } } } From f68084a629f59cde1e1c287c1532e1ce46df040a Mon Sep 17 00:00:00 2001 From: Seth Pellegrino Date: Thu, 18 May 2023 21:53:26 -0700 Subject: [PATCH 2/4] lint: make clippy happy Replaces `get(0)` with `first()` as suggested by https://rust-lang.github.io/rust-clippy/master/index.html#get_first --- src/util.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/util.rs b/src/util.rs index 6d06bd2c..fc58314c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -232,7 +232,7 @@ pub trait ToSanitizedCase { impl ToSanitizedCase for str { fn to_sanitized_pascal_case(&self) -> Cow { let s = Case::Pascal.sanitize(self); - if s.as_bytes().get(0).unwrap_or(&0).is_ascii_digit() { + if s.as_bytes().first().unwrap_or(&0).is_ascii_digit() { Cow::from(format!("_{}", s)) } else { s @@ -240,7 +240,7 @@ impl ToSanitizedCase for str { } fn to_sanitized_constant_case(&self) -> Cow { let s = Case::Constant.sanitize(self); - if s.as_bytes().get(0).unwrap_or(&0).is_ascii_digit() { + if s.as_bytes().first().unwrap_or(&0).is_ascii_digit() { Cow::from(format!("_{}", s)) } else { s @@ -250,7 +250,7 @@ impl ToSanitizedCase for str { const INTERNALS: [&str; 4] = ["set_bit", "clear_bit", "bit", "bits"]; let s = Case::Snake.sanitize(self); - if s.as_bytes().get(0).unwrap_or(&0).is_ascii_digit() { + if s.as_bytes().first().unwrap_or(&0).is_ascii_digit() { format!("_{}", s).into() } else if INTERNALS.contains(&s.as_ref()) { s + "_" From 328d0c37e21379cbcbf4952bad52793ee78be97f Mon Sep 17 00:00:00 2001 From: Seth Pellegrino Date: Thu, 18 May 2023 21:56:42 -0700 Subject: [PATCH 3/4] docs: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 16b1f482..6f2ca0b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/). - Optimize case change/sanitize - Fix dangling implicit derives - Fix escaping <> and & characters in doc attributes +- Add `interrupt_link_section` config parameter for controlling the `#[link_section = "..."]` attribute of `__INTERRUPTS` ## [v0.28.0] - 2022-12-25 From 1c773fbf6abca4cf8117de5d4d93d7777937d137 Mon Sep 17 00:00:00 2001 From: Seth Pellegrino Date: Fri, 19 May 2023 08:12:12 -0700 Subject: [PATCH 4/4] refactor: use `Option::map` instead of if-let Thanks to @burrbull for the review! --- src/generate/interrupt.rs | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/generate/interrupt.rs b/src/generate/interrupt.rs index 6ea1785f..92439ff7 100644 --- a/src/generate/interrupt.rs +++ b/src/generate/interrupt.rs @@ -188,13 +188,11 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);")?; } - let link_section_attr = if let Some(section) = &config.interrupt_link_section { + let link_section_attr = config.interrupt_link_section.as_ref().map(|section| { quote! { #[link_section = #section] } - } else { - quote! {} - }; + }); root.extend(quote! { #[cfg(feature = "rt")] @@ -222,13 +220,11 @@ pub fn render( writeln!(device_x, "PROVIDE({name} = DefaultHandler);")?; } - let link_section_attr = if let Some(section) = &config.interrupt_link_section { + let link_section_attr = config.interrupt_link_section.as_ref().map(|section| { quote! { #[link_section = #section] } - } else { - quote! {} - }; + }); root.extend(quote! { #[cfg(feature = "rt")]