From 9370dd3946caa89d90bd04e4da8d43f374f9d29e Mon Sep 17 00:00:00 2001 From: Johnathan Van Why Date: Tue, 15 Feb 2022 13:27:47 -0800 Subject: [PATCH 1/2] Migrate the ARM entry point to `global_asm!`. Building `libtock_runtime` no longer requires an external toolchain. Help Needed ----------- This generates significantly different code for the entry point than the old implementation. I don't have an ARM board to test with, or enough knowledge to understand why the difference exists, so I need help testing/debugging this. --- .github/workflows/artifacts.yml | 1 - .github/workflows/ci.yml | 2 +- .github/workflows/mac-os.yml | 5 - .github/workflows/size-diff.yml | 1 - runtime/build.rs | 35 ++--- runtime/extern_asm.rs | 130 ------------------ .../{asm/asm_arm.S => src/startup/asm_arm.s} | 0 runtime/src/startup/mod.rs | 3 +- 8 files changed, 18 insertions(+), 159 deletions(-) delete mode 100644 runtime/extern_asm.rs rename runtime/{asm/asm_arm.S => src/startup/asm_arm.s} (100%) diff --git a/.github/workflows/artifacts.yml b/.github/workflows/artifacts.yml index de93dae6..579b6c32 100644 --- a/.github/workflows/artifacts.yml +++ b/.github/workflows/artifacts.yml @@ -12,7 +12,6 @@ jobs: - name: Install dependencies run: | - sudo apt-get install binutils-arm-none-eabi cargo install elf2tab - name: Build LEDs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e4b7a8a6..fd42e093 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,7 +42,7 @@ jobs: # relocation. - name: Build and Test run: | - sudo apt-get install binutils-arm-none-eabi ninja-build + sudo apt-get install ninja-build cd "${GITHUB_WORKSPACE}" echo "[target.'cfg(all())']" >> .cargo/config echo 'rustflags = ["-D", "warnings"]' >> .cargo/config diff --git a/.github/workflows/mac-os.yml b/.github/workflows/mac-os.yml index 025c231a..de7a8dd4 100644 --- a/.github/workflows/mac-os.yml +++ b/.github/workflows/mac-os.yml @@ -16,11 +16,6 @@ jobs: - name: Clone repository uses: actions/checkout@v2 - - name: Install arm-none-eabi-gcc - uses: fiam/arm-none-eabi-gcc@v1 - with: - release: '10-2020-q4' - - name: Build and Test run: | cd "${GITHUB_WORKSPACE}" diff --git a/.github/workflows/size-diff.yml b/.github/workflows/size-diff.yml index d614efdc..6989b392 100644 --- a/.github/workflows/size-diff.yml +++ b/.github/workflows/size-diff.yml @@ -39,7 +39,6 @@ jobs: # master. - name: Compute sizes run: | - sudo apt-get install binutils-arm-none-eabi UPSTREAM_REMOTE_NAME="${UPSTREAM_REMOTE_NAME:-origin}" GITHUB_BASE_REF="${GITHUB_BASE_REF:-master}" cd "${GITHUB_WORKSPACE}" diff --git a/runtime/build.rs b/runtime/build.rs index 9bddacaa..0e3561e0 100644 --- a/runtime/build.rs +++ b/runtime/build.rs @@ -1,11 +1,9 @@ -mod extern_asm; - // auto_layout() identifies the correct linker scripts to use based on the // LIBTOCK_PLATFORM environment variable, and copies the linker scripts into // OUT_DIR. The cargo invocation must pass -C link-arg=-Tlayout.ld to rustc // (using the rustflags cargo config). #[cfg(not(feature = "no_auto_layout"))] -fn auto_layout(out_dir: &str) { +fn auto_layout() { use std::fs::copy; use std::path::PathBuf; @@ -18,6 +16,16 @@ fn auto_layout(out_dir: &str) { // Read configuration from environment variables. + // Note: cargo fails if run in a path that is not valid Unicode, so this + // script doesn't need to handle non-Unicode paths. Also, OUT_DIR cannot be + // in a location with a newline in it, or we have no way to pass + // rustc-link-search to cargo. + let out_dir = &std::env::var("OUT_DIR").expect("Unable to read OUT_DIR"); + assert!( + !out_dir.contains('\n'), + "Build path contains a newline, which is unsupported" + ); + // Read the platform environment variable as a String (our platform names // should all be valid UTF-8). let platform = std::env::var(PLATFORM_CFG_VAR).expect("Please specify LIBTOCK_PLATFORM"); @@ -35,25 +43,12 @@ fn auto_layout(out_dir: &str) { println!("cargo:rerun-if-changed={}", LAYOUT_GENERIC_FILENAME); copy(LAYOUT_GENERIC_FILENAME, out_layout_generic) .expect("Unable to copy layout_generic.ld into OUT_DIR"); + + // Tell rustc where to search for the layout file. + println!("cargo:rustc-link-search={}", out_dir); } fn main() { - // Note: cargo fails if run in a path that is not valid Unicode, so this - // script doesn't need to handle non-Unicode paths. Also, OUT_DIR cannot be - // in a location with a newline in it, or we have no way to pass - // rustc-link-search to cargo. - let out_dir = &std::env::var("OUT_DIR").expect("Unable to read OUT_DIR"); - assert!( - !out_dir.contains('\n'), - "Build path contains a newline, which is unsupported" - ); - #[cfg(not(feature = "no_auto_layout"))] - auto_layout(out_dir); - - extern_asm::build_and_link(out_dir); - - // This link search path is used by both auto_layout() and - // extern_asm::build_and_link(). - println!("cargo:rustc-link-search={}", out_dir); + auto_layout(); } diff --git a/runtime/extern_asm.rs b/runtime/extern_asm.rs deleted file mode 100644 index df2fa56e..00000000 --- a/runtime/extern_asm.rs +++ /dev/null @@ -1,130 +0,0 @@ -//! Build script module for compiling the external assembly (used for the entry -//! point) and linking it into the process binary. Requires out_dir to be added -//! to rustc's link search path. - -/// Compiles the external assembly and tells cargo/rustc to link the resulting -/// library into the crate. Panics if it is unable to find a working assembly -/// toolchain or if the assembly fails to compile. -pub(crate) fn build_and_link(out_dir: &str) { - use std::env::var; - let arch = var("CARGO_CFG_TARGET_ARCH").expect("Unable to read CARGO_CFG_TARGET_ARCH"); - - // Identify the toolchain configurations to try for the target architecture. - // We support trying multiple toolchains because not all toolchains are - // available on every OS that we want to support development on. - let build_configs: &[AsmBuildConfig] = match arch.as_str() { - "arm" => &[AsmBuildConfig { - prefix: "arm-none-eabi", - as_extra_args: &[], - strip: false, - }], - // RISC-V's entry point is compiled using global_asm! rather than an - // external toolchain. - "riscv32" => return, - unknown_arch => { - panic!("Unsupported architecture {}", unknown_arch); - } - }; - - // Loop through toolchain configs until one works. - for &build_config in build_configs { - if try_build(&arch, build_config, out_dir).is_ok() { - return; - } - } - - panic!("Unable to find a toolchain for architecture {}", arch); -} - -#[derive(Clone, Copy)] -struct AsmBuildConfig { - // Prefix, which is prepended to the command names. - prefix: &'static str, - - // Extra arguments to pass to the assembler. - as_extra_args: &'static [&'static str], - - // Do we need to strip the object file before packing it into the library - // archive? This should be set to true on platforms where the assembler adds - // local symbols to the object file. - strip: bool, -} - -// Indicates the toolchain in the build config is unavailable. -struct ToolchainUnavailable; - -fn try_build( - arch: &str, - build_config: AsmBuildConfig, - out_dir: &str, -) -> Result<(), ToolchainUnavailable> { - use std::path::PathBuf; - use std::process::Command; - - // Invoke the assembler to produce an object file. - let asm_source = &format!("asm/asm_{}.S", arch); - let obj_file_path = [out_dir, "libtock_rt_asm.o"].iter().collect::(); - let obj_file = obj_file_path.to_str().expect("Non-Unicode obj_file_path"); - let as_result = Command::new(format!("{}-as", build_config.prefix)) - .args(build_config.as_extra_args) - .args(&[asm_source, "-o", obj_file]) - .status(); - - match as_result { - Err(error) => { - if error.kind() == std::io::ErrorKind::NotFound { - // This `as` command does not exist. Return an error so - // build_an_link can try another config (if one is available). - return Err(ToolchainUnavailable); - } else { - panic!("Error invoking assembler: {}", error); - } - } - Ok(status) => { - assert!(status.success(), "Assembler returned an error"); - } - } - - // At this point, we know this toolchain is installed. We will fail if later - // commands are uninstalled rather than trying a different build config. - - println!("cargo:rerun-if-changed={}", asm_source); - - // Run `strip` if necessary. - if build_config.strip { - let strip_cmd = format!("{}-strip", build_config.prefix); - let status = Command::new(&strip_cmd) - .args(&["-K", "start", "-K", "rust_start", obj_file]) - .status() - .unwrap_or_else(|_| panic!("Failed to invoke {}", strip_cmd)); - assert!(status.success(), "{} returned an error", strip_cmd); - } - - // Remove the archive file in case there is something unexpected in it. This - // prevents issues from persisting across invocations of this script. - const ARCHIVE_NAME: &str = "tock_rt_asm"; - let archive_path: PathBuf = [out_dir, &format!("lib{}.a", ARCHIVE_NAME)] - .iter() - .collect(); - if let Err(error) = std::fs::remove_file(&archive_path) { - if error.kind() != std::io::ErrorKind::NotFound { - panic!("Unable to remove archive file {}", archive_path.display()); - } - } - - // Create the library archive. - let ar_cmd = format!("{}-ar", build_config.prefix); - let archive = archive_path.to_str().expect("Non-Unicode archive_path"); - let status = std::process::Command::new(&ar_cmd) - // c == Do not complain if archive needs to be created. - // r == Insert or replace file in archive. - .args(&["cr", archive, obj_file]) - .status() - .unwrap_or_else(|_| panic!("Failed to invoke {}", ar_cmd)); - assert!(status.success(), "{} returned an error", ar_cmd); - - // Tell rustc to link the binary against the library archive. - println!("cargo:rustc-link-lib=static={}", ARCHIVE_NAME); - - Ok(()) -} diff --git a/runtime/asm/asm_arm.S b/runtime/src/startup/asm_arm.s similarity index 100% rename from runtime/asm/asm_arm.S rename to runtime/src/startup/asm_arm.s diff --git a/runtime/src/startup/mod.rs b/runtime/src/startup/mod.rs index 0de80f6c..d618287b 100644 --- a/runtime/src/startup/mod.rs +++ b/runtime/src/startup/mod.rs @@ -2,7 +2,8 @@ // Include the correct `start` symbol (the program entry point) for the // architecture. -// TODO: Migrate ARM to global_asm! and delete extern_asm.rs. +#[cfg(target_arch = "arm")] +core::arch::global_asm!(include_str!("asm_arm.s")); #[cfg(target_arch = "riscv32")] core::arch::global_asm!(include_str!("asm_riscv32.s")); From 8321d1af041777c8891df90674862acb2147051f Mon Sep 17 00:00:00 2001 From: Johnathan Van Why Date: Tue, 15 Feb 2022 14:36:22 -0800 Subject: [PATCH 2/2] Tweak the ARM assembly to generate smaller instructions. LLVM doesn't automatically pick the smallest instructions for the add/mov/sub mnemonics like GCC did. This means that moving to global_asm! made `start` larger. This change explicitly names the smaller instructions, and generates code of identical size to the GCC toolchain. There is one remaining difference between the external assembly and this `global_asm!` implementation: LLVM translates `mov r5, r0` into `mov r5, r0`, whereas GCC translated it into `adds r5, r0, #0`. --- runtime/src/startup/asm_arm.s | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/runtime/src/startup/asm_arm.s b/runtime/src/startup/asm_arm.s index 349b4162..5a157b31 100644 --- a/runtime/src/startup/asm_arm.s +++ b/runtime/src/startup/asm_arm.s @@ -36,21 +36,21 @@ start: mov r4, pc /* r4 = address of .start + 4 (Thumb bit unset) */ mov r5, r0 /* Save rt_header; we use r0 for syscalls */ ldr r0, [r5, #0] /* r0 = rt_header.start */ - add r0, #3 /* r0 = rt_header.start + 4 - 1 (for Thumb bit) */ + adds r0, #3 /* r0 = rt_header.start + 4 - 1 (for Thumb bit) */ cmp r0, r4 beq .Lset_brk /* Skip error handling if pc correct */ /* If the beq on the previous line did not jump, then the binary is not at * the correct location. Report the error via LowLevelDebug then exit. */ - mov r0, #8 /* LowLevelDebug driver number */ - mov r1, #1 /* Command: print alert code */ - mov r2, #2 /* Alert code 2 (incorrect location */ - svc 2 /* Execute `command` */ - mov r0, #0 /* Operation: exit-terminate */ - svc 6 /* Execute `exit` */ + movs r0, #8 /* LowLevelDebug driver number */ + movs r1, #1 /* Command: print alert code */ + movs r2, #2 /* Alert code 2 (incorrect location */ + svc 2 /* Execute `command` */ + movs r0, #0 /* Operation: exit-terminate */ + svc 6 /* Execute `exit` */ .Lset_brk: /* memop(): set brk to rt_header's initial break value */ - mov r0, #0 /* operation: set break */ + movs r0, #0 /* operation: set break */ ldr r1, [r5, #4] /* rt_header`s initial process break */ svc 5 /* call `memop` */ @@ -66,9 +66,9 @@ start: .Ldata_loop_body: ldr r3, [r1] /* r3 = *src */ str r3, [r2] /* *(dest) = r3 */ - sub r0, #4 /* remaining -= 4 */ - add r1, #4 /* src += 4 */ - add r2, #4 /* dest += 4 */ + subs r0, #4 /* remaining -= 4 */ + adds r1, #4 /* src += 4 */ + adds r2, #4 /* dest += 4 */ cmp r0, #0 bne .Ldata_loop_body /* Iterate again if remaining != 0 */ @@ -76,11 +76,11 @@ start: ldr r0, [r5, #24] /* remaining = rt_header.bss_size */ cbz r0, .Lcall_rust_start /* Jump to call_rust_start if remaining == 0 */ ldr r1, [r5, #28] /* dest = rt_header.bss_start */ - mov r2, #0 /* r2 = 0 */ + movs r2, #0 /* r2 = 0 */ .Lbss_loop_body: strb r2, [r1] /* *(dest) = r2 = 0 */ - sub r0, #1 /* remaining -= 1 */ - add r1, #1 /* dest += 1 */ + subs r0, #1 /* remaining -= 1 */ + adds r1, #1 /* dest += 1 */ cmp r0, #0 bne .Lbss_loop_body /* Iterate again if remaining != 0 */