From 55811d8dac9109aa6357639908fb4f6c479480f9 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 23 May 2021 21:51:10 -0700 Subject: [PATCH] stage2: introduce clangAssemblerSupportsMcpuArg Clang has a completely inconsistent CLI for its integrated assembler for each target architecture. For x86_64, for example, it does not accept an -mcpu parameter, and emits "warning: unused parameter". However, for ARM, -mcpu is needed in order to properly lower assembly to machine code instructions (see new standalone test case provided thanks to @g-w1). This is a compromise between b8f85a805bf61ae11d6ee2bd6d8356fbc98ee3ba and afb9f695b1bdbf81185e7d55d5783bcbab880989. --- src/Compilation.zig | 42 ++++++++++++++++++---------- src/target.zig | 9 ++++++ test/standalone.zig | 1 + test/standalone/issue_8550/boot.S | 33 ++++++++++++++++++++++ test/standalone/issue_8550/build.zig | 21 ++++++++++++++ test/standalone/issue_8550/linker.ld | 42 ++++++++++++++++++++++++++++ test/standalone/issue_8550/main.zig | 6 ++++ 7 files changed, 139 insertions(+), 15 deletions(-) create mode 100644 test/standalone/issue_8550/boot.S create mode 100644 test/standalone/issue_8550/build.zig create mode 100644 test/standalone/issue_8550/linker.ld create mode 100644 test/standalone/issue_8550/main.zig diff --git a/src/Compilation.zig b/src/Compilation.zig index 405066331f..023a54ca8d 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -2943,26 +2943,38 @@ pub fn addCCArgs( try argv.append("-fPIC"); } }, - .shared_library, .assembly, .ll, .bc, .unknown, .static_library, .object, .zig => {}, + .shared_library, .ll, .bc, .unknown, .static_library, .object, .zig => {}, + .assembly => { + // The Clang assembler does not accept the list of CPU features like the + // compiler frontend does. Therefore we must hard-code the -m flags for + // all CPU features here. + switch (target.cpu.arch) { + .riscv32, .riscv64 => { + if (std.Target.riscv.featureSetHas(target.cpu.features, .relax)) { + try argv.append("-mrelax"); + } else { + try argv.append("-mno-relax"); + } + }, + else => { + // TODO + }, + } + if (target_util.clangAssemblerSupportsMcpuArg(target)) { + if (target.cpu.model.llvm_name) |llvm_name| { + try argv.append(try std.fmt.allocPrint(arena, "-mcpu={s}", .{llvm_name})); + } + } + }, } if (out_dep_path) |p| { try argv.appendSlice(&[_][]const u8{ "-MD", "-MV", "-MF", p }); } - // Argh, why doesn't the assembler accept the list of CPU features?! - // I don't see a way to do this other than hard coding everything. - switch (target.cpu.arch) { - .riscv32, .riscv64 => { - if (std.Target.riscv.featureSetHas(target.cpu.features, .relax)) { - try argv.append("-mrelax"); - } else { - try argv.append("-mno-relax"); - } - }, - else => { - // TODO - }, - } + // We never want clang to invoke the system assembler for anything. So we would want + // this option always enabled. However, it only matters for some targets. To avoid + // "unused parameter" warnings, and to keep CLI spam to a minimum, we only put this + // flag on the command line if it is necessary. if (target_util.clangMightShellOutForAssembly(target)) { try argv.append("-integrated-as"); } diff --git a/src/target.zig b/src/target.zig index e077d9629c..b4e6123819 100644 --- a/src/target.zig +++ b/src/target.zig @@ -389,3 +389,12 @@ pub fn clangMightShellOutForAssembly(target: std.Target) bool { // when targeting a non-BSD OS. return target.cpu.arch.isSPARC(); } + +/// Each backend architecture in Clang has a different codepath which may or may not +/// support an -mcpu flag. +pub fn clangAssemblerSupportsMcpuArg(target: std.Target) bool { + return switch (target.cpu.arch) { + .arm, .armeb, .thumb, .thumbeb => true, + else => false, + }; +} diff --git a/test/standalone.zig b/test/standalone.zig index 9dd849ef88..f2251e5204 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -16,6 +16,7 @@ pub fn addCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/standalone/link_interdependent_static_c_libs/build.zig"); cases.addBuildFile("test/standalone/link_static_lib_as_system_lib/build.zig"); cases.addBuildFile("test/standalone/issue_339/build.zig"); + cases.addBuildFile("test/standalone/issue_8550/build.zig"); cases.addBuildFile("test/standalone/issue_794/build.zig"); cases.addBuildFile("test/standalone/issue_5825/build.zig"); cases.addBuildFile("test/standalone/pkg_import/build.zig"); diff --git a/test/standalone/issue_8550/boot.S b/test/standalone/issue_8550/boot.S new file mode 100644 index 0000000000..20fd01eefa --- /dev/null +++ b/test/standalone/issue_8550/boot.S @@ -0,0 +1,33 @@ + .section ".text.boot" + + .global _start + + _start: + mrc p15, #0, r1, c0, c0, #5 + and r1, r1, #3 + cmp r1, #0 + bne halt + + mov sp, #0x8000 + + ldr r4, =__bss_start + ldr r9, =__bss_end + mov r5, #0 + mov r6, #0 + mov r7, #0 + mov r8, #0 + b 2f + + 1: + stmia r4!, {r5-r8} + + 2: + cmp r4, r9 + blo 1b + + ldr r3, =main + blx r3 + + halt: + wfe + b halt diff --git a/test/standalone/issue_8550/build.zig b/test/standalone/issue_8550/build.zig new file mode 100644 index 0000000000..6f01846249 --- /dev/null +++ b/test/standalone/issue_8550/build.zig @@ -0,0 +1,21 @@ +const std = @import("std"); + +pub fn build(b: *std.build.Builder) !void { + const target = std.zig.CrossTarget{ + .os_tag = .freestanding, + .cpu_arch = .arm, + .cpu_model = .{ + .explicit = &std.Target.arm.cpu.arm1176jz_s, + }, + }; + const mode = b.standardReleaseOptions(); + const kernel = b.addExecutable("kernel", "./main.zig"); + kernel.addObjectFile("./boot.S"); + kernel.setLinkerScriptPath("./linker.ld"); + kernel.setBuildMode(mode); + kernel.setTarget(target); + kernel.install(); + + const test_step = b.step("test", "Test it"); + test_step.dependOn(&kernel.step); +} diff --git a/test/standalone/issue_8550/linker.ld b/test/standalone/issue_8550/linker.ld new file mode 100644 index 0000000000..9c6c96eea9 --- /dev/null +++ b/test/standalone/issue_8550/linker.ld @@ -0,0 +1,42 @@ +ENTRY(_start) + +SECTIONS +{ + /* Starts at LOADER_ADDR. */ + . = 0x8000; + __start = .; + __text_start = .; + .text : + { + KEEP(*(.text.boot)) + *(.text) + } + . = ALIGN(4096); /* align to page size */ + __text_end = .; + + __rodata_start = .; + .rodata : + { + *(.rodata) + } + . = ALIGN(4096); /* align to page size */ + __rodata_end = .; + + __data_start = .; + .data : + { + *(.data) + } + . = ALIGN(4096); /* align to page size */ + __data_end = .; + + __bss_start = .; + .bss : + { + bss = .; + *(.bss) + } + . = ALIGN(4096); /* align to page size */ + __bss_end = .; + __end = .; +} diff --git a/test/standalone/issue_8550/main.zig b/test/standalone/issue_8550/main.zig new file mode 100644 index 0000000000..7dae7f856c --- /dev/null +++ b/test/standalone/issue_8550/main.zig @@ -0,0 +1,6 @@ +export fn main(r0: u32, r1: u32, atags: u32) callconv(.C) noreturn { + unreachable; // never gets run so it doesn't matter +} +pub fn panic(msg: []const u8, error_return_trace: ?*@import("std").builtin.StackTrace) noreturn { + while (true) {} +}