Cleaner implementation of LLVM patch and changelog

Author: romancardenas

.github/workflows/riscv-rt.yaml

@@ -14,8 +14,10 @@ jobs:
         toolchain: [ stable, nightly, 1.61.0 ]
         target:
           - riscv32i-unknown-none-elf
+          - riscv32im-unknown-none-elf
           - riscv32imc-unknown-none-elf
           - riscv32imac-unknown-none-elf
+          - riscv32imafc-unknown-none-elf
           - riscv64imac-unknown-none-elf
           - riscv64gc-unknown-none-elf
         example:

riscv-rt/CHANGELOG.md

@@ -9,8 +9,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ### Changed
 
+- Limit rustc cfg flags to `riscvi`, `riscve`, `riscvm`, `riscvf`, and `riscvd`.
+- Temporary use of `RISCV_RT_LLVM_ARCH_PATCH` environment variable to include the
+  temporary patch required for avoid LLVM spurious errors.
+- `riscv-rt` now use the `RISCV_RT_BASE_ISA` environment variable to configure the behavior
+  of `riscv-rt-macros` depending on aspects of the base ISA (e.g., RV32I or RV32E).
+- Use `riscv-target-parser` in build script to identify target-specific configurations.
 - Add documentation to trap frame fields.
-- Avoid using `t3`+ in startup assembly to ensure compatibility with RVE32.
+- Avoid using `t3`+ in startup assembly to ensure compatibility with RVE.
 - `link.x.in`: remove references to `eh_frame`.
 - Rename start/end section symbols to align with `cortex-m-rt`:
     - `_stext`: it remains, as linker files can modify it.

riscv-rt/build.rs

@@ -1,8 +1,11 @@
 // NOTE: Adapted from cortex-m/build.rs
 
-use riscv_target_parser::{Extension, RiscvTarget, Width};
+use riscv_target_parser::RiscvTarget;
 use std::{env, fs, io, path::PathBuf};
 
+// List of all possible RISC-V configurations to check for in risv-rt
+const RISCV_CFG: [&str; 5] = ["riscvi", "riscve", "riscvm", "riscvf", "riscvd"];
+
 fn add_linker_script(arch_width: u32) -> io::Result<()> {
     // Read the file to a string and replace all occurrences of ${ARCH_WIDTH} with the arch width
     let mut content = fs::read_to_string("link.x.in")?;
@@ -20,32 +23,36 @@ fn add_linker_script(arch_width: u32) -> io::Result<()> {
 
 fn main() {
     // Required until target_feature risc-v is stable and in-use (rust 1.75)
-    for ext in ['i', 'e', 'm', 'a', 'f', 'd', 'g', 'c'] {
-        println!("cargo:rustc-check-cfg=cfg(riscv{})", ext);
+    for ext in RISCV_CFG.iter() {
+        println!("cargo:rustc-check-cfg=cfg({ext})");
     }
 
     let target = env::var("TARGET").unwrap();
     let cargo_flags = env::var("CARGO_ENCODED_RUSTFLAGS").unwrap();
 
     if let Ok(target) = RiscvTarget::build(&target, &cargo_flags) {
         let width = target.width();
-        let base = target.base_extension().expect("No base extension found");
-
-        // set environmet variable RISCV_RT_BASE_ISA to the width of the target
-        // this is used in riscv_rt_macros to determine the base ISA
-        let env_var = match (width, base) {
-            (Width::W32, Extension::I) => "rv32i",
-            (Width::W32, Extension::E) => "rv32e",
-            (Width::W64, Extension::I) => "rv64i",
-            (Width::W64, Extension::E) => "rv64e",
-            _ => panic!("Unsupported target"),
-        };
-        println!("cargo:rustc-env=RISCV_RT_BASE_ISA={env_var}");
+
+        // set environmet variable RISCV_RT_BASE_ISA to the base ISA of the target.
+        println!(
+            "cargo:rustc-env=RISCV_RT_BASE_ISA={}",
+            target.llvm_base_isa()
+        );
+        // set environment variable RISCV_RT_LLVM_ARCH_PATCH to patch LLVM bug.
+        // (this env variable is temporary and will be removed after LLVM being fixed)
+        println!(
+            "cargo:rustc-env=RISCV_RT_LLVM_ARCH_PATCH={}",
+            target.llvm_arch_patch()
+        );
+        // make sure that these env variables are not changed without notice.
+        println!("cargo:rerun-if-env-changed=RISCV_RT_BASE_ISA");
+        println!("cargo:rerun-if-env-changed=RISCV_RT_LLVM_ARCH_PATCH");
 
         for flag in target.rustc_flags() {
             // Required until target_feature risc-v is stable and in-use
-            println!("cargo:rustc-check-cfg=cfg({flag})");
-            println!("cargo:rustc-cfg={flag}");
+            if RISCV_CFG.contains(&flag.as_str()) {
+                println!("cargo:rustc-cfg={flag}");
+            }
         }
         add_linker_script(width.into()).unwrap();
     }

riscv-rt/macros/src/lib.rs

@@ -452,6 +452,18 @@ fn load_trap(arch: RiscvArch) -> String {
         .join("\n    ")
 }
 
+/// Temporary patch macro to deal with LLVM bug
+#[proc_macro]
+pub fn llvm_arch_patch(_input: TokenStream) -> TokenStream {
+    let q = if let Ok(arch) = std::env::var("RISCV_RT_LLVM_ARCH_PATCH") {
+        let patch = format!(".attribute arch,\"{arch}\"");
+        quote! { core::arch::global_asm!{#patch} }
+    } else {
+        quote!(compile_error!("RISCV_RT_LLVM_ARCH_PATCH is not set"))
+    };
+    q.into()
+}
+
 /// Generates weak `_start_trap` function in assembly.
 ///
 /// This implementation stores all registers in the trap frame and calls `_start_trap_rust`.

riscv-rt/src/asm.rs

@@ -29,15 +29,7 @@ macro_rules! cfg_global_asm {
 // - https://github.com/rust-embedded/riscv/issues/175
 // - https://github.com/rust-lang/rust/issues/80608
 // - https://github.com/llvm/llvm-project/issues/61991
-cfg_global_asm!(
-    "// Provisional patch to avoid LLVM spurious errors when compiling in release mode.",
-    #[cfg(all(target_arch = "riscv32", riscvm))]
-    ".attribute arch, \"rv32im\"",
-    #[cfg(all(target_arch = "riscv64", riscvm, not(riscvg)))]
-    ".attribute arch, \"rv64im\"",
-    #[cfg(all(target_arch = "riscv64", riscvg))]
-    ".attribute arch, \"rv64g\"",
-);
+riscv_rt_macros::llvm_arch_patch!();
 
 // Entry point of all programs (_start). It initializes DWARF call frame information,
 // the stack pointer, the frame pointer (needed for closures to work in start_rust)

riscv-target-parser/Cargo.toml

@@ -7,6 +7,6 @@ authors = ["The RISC-V Team <risc-v@teams.rust-embedded.org>"]
 categories = ["embedded", "no-std"]
 description = "Parser for RISC-V target specifications"
 documentation = "https://docs.rs/riscv-target-parser"
-keywords = ["riscv", "runtime", "startup"]
+keywords = ["riscv", "build"]
 license = "ISC"
 edition = "2021"

riscv-target-parser/src/lib.rs

@@ -162,16 +162,48 @@ impl RiscvTarget {
     /// }
     ///
     pub fn rustc_flags(&self) -> Vec<String> {
-        let mut res = self
-            .extensions
+        self.extensions
             .extensions()
             .iter()
             .map(|e| format!("riscv{e}"))
-            .collect::<Vec<_>>();
-        if self.extensions.is_g() {
-            res.push("riscvg".to_string());
+            .collect::<Vec<_>>()
+    }
+
+    /// Returns the LLVM base ISA for the given RISC-V target.
+    pub fn llvm_base_isa(&self) -> String {
+        match (self.width, self.extensions.base_extension()) {
+            (Width::W32, Some(Extension::I)) => String::from("rv32i"),
+            (Width::W32, Some(Extension::E)) => String::from("rv32e"),
+            (Width::W64, Some(Extension::I)) => String::from("rv64i"),
+            (Width::W64, Some(Extension::E)) => String::from("rv64e"),
+            (_, None) => panic!("RISC-V target must have a base extension"),
+            _ => panic!("LLVM does not support this base ISA"),
+        }
+    }
+
+    /// Returns the arch code to patch LLVM spurious errors.
+    ///
+    /// # Note
+    ///
+    /// This is a provisional patch and is limited to work for the riscv-rt crate only.
+    ///
+    /// # Related issues
+    ///
+    /// - https://github.com/rust-embedded/riscv/issues/175
+    /// - https://github.com/rust-lang/rust/issues/80608
+    /// - https://github.com/llvm/llvm-project/issues/61991
+    pub fn llvm_arch_patch(&self) -> String {
+        let mut patch = self.llvm_base_isa();
+        if self.extensions.contains(&Extension::M) {
+            patch.push('m');
+        }
+        if self.extensions.contains(&Extension::F) {
+            patch.push('f');
+        }
+        if self.extensions.contains(&Extension::D) {
+            patch.push('d');
         }
-        res
+        patch
     }
 
     /// Returns the width of the RISC-V architecture.