From 40104f2145c3b7e2725604c0c777646a5fb621aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Tue, 15 Oct 2024 20:34:56 +0200 Subject: [PATCH 1/6] compiler: Disallow function alignment for nvptx and spirv. --- src/target.zig | 9 ++++++++- .../function_alignment_on_unsupported_target.zig | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/cases/compile_errors/function_alignment_on_unsupported_target.zig diff --git a/src/target.zig b/src/target.zig index 95c9798f65..4b9d9af722 100644 --- a/src/target.zig +++ b/src/target.zig @@ -487,7 +487,14 @@ pub fn minFunctionAlignment(target: std.Target) Alignment { pub fn supportsFunctionAlignment(target: std.Target) bool { return switch (target.cpu.arch) { - .wasm32, .wasm64 => false, + .nvptx, + .nvptx64, + .spirv, + .spirv32, + .spirv64, + .wasm32, + .wasm64, + => false, else => true, }; } diff --git a/test/cases/compile_errors/function_alignment_on_unsupported_target.zig b/test/cases/compile_errors/function_alignment_on_unsupported_target.zig new file mode 100644 index 0000000000..7b033e0f60 --- /dev/null +++ b/test/cases/compile_errors/function_alignment_on_unsupported_target.zig @@ -0,0 +1,7 @@ +export fn entry() align(0) void {} + +// error +// backend=stage2 +// target=nvptx-cuda,nvptx64-cuda,spirv-vulkan,spirv32-opencl,spirv64-opencl,wasm32-freestanding,wasm64-freestanding +// +// :1:25: error: target does not support function alignment From d2f04e919c833288d5cf1efa97c25bbaae101168 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Tue, 15 Oct 2024 20:35:56 +0200 Subject: [PATCH 2/6] compiler: Update defaultFunctionAlignment()/minFunctionAlignment() for more targets. defaultFunctionAlignment() can be made more sophisticated over time based on the CPU model and/or features. For now, I've picked some reasonable values for the CPUs that are most commonly used in practice. (Values are sourced from LLVM.) --- src/target.zig | 61 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 53 insertions(+), 8 deletions(-) diff --git a/src/target.zig b/src/target.zig index 4b9d9af722..cce1787f33 100644 --- a/src/target.zig +++ b/src/target.zig @@ -461,26 +461,71 @@ pub fn llvmMachineAbi(target: std.Target) ?[:0]const u8 { /// This function returns 1 if function alignment is not observable or settable. pub fn defaultFunctionAlignment(target: std.Target) Alignment { + // Overrides of the minimum for performance. return switch (target.cpu.arch) { - .arm, .armeb => .@"4", - .aarch64, .aarch64_be => .@"4", - .sparc, .sparc64 => .@"4", - .riscv64 => .@"2", - else => .@"1", + .csky, + .thumb, + .thumbeb, + .xcore, + => .@"4", + .aarch64, + .aarch64_be, + .hexagon, + .powerpc, + .powerpcle, + .powerpc64, + .powerpc64le, + .s390x, + .x86, + .x86_64, + => .@"16", + .loongarch32, + .loongarch64, + => .@"32", + else => minFunctionAlignment(target), }; } +/// This function returns 1 if function alignment is not observable or settable. pub fn minFunctionAlignment(target: std.Target) Alignment { return switch (target.cpu.arch) { + .riscv32, + .riscv64, + => if (std.Target.riscv.featureSetHasAny(target.cpu.features, .{ .c, .zca })) .@"2" else .@"4", + .thumb, + .thumbeb, + .csky, + .m68k, + .msp430, + .s390x, + .xcore, + => .@"2", + .arc, .arm, .armeb, .aarch64, .aarch64_be, - .riscv32, - .riscv64, + .hexagon, + .lanai, + .loongarch32, + .loongarch64, + .mips, + .mipsel, + .powerpc, + .powerpcle, + .powerpc64, + .powerpc64le, .sparc, .sparc64, - => .@"2", + .xtensa, + => .@"4", + .bpfel, + .bpfeb, + .mips64, + .mips64el, + => .@"8", + .ve, + => .@"16", else => .@"1", }; } From ef72b91ac2b96ba64a53b08a661e9ad83a828ee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Tue, 15 Oct 2024 21:28:42 +0200 Subject: [PATCH 3/6] compiler: Remove uses of defaultFunctionAlignment() in the frontend. minFunctionAlignment() is something we can know ahead of time for any given target because it's a matter of ABI. However, defaultFunctionAlignment() is a matter of optimization and every backend can do it differently depending on any number of factors. For example, LLVM will base the choice on the CPU model in its aarch64 backend. So just don't use this value in the frontend. --- src/Sema.zig | 8 ++------ src/Type.zig | 2 +- src/target.zig | 3 ++- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index 1250a06a9c..a2a13184e4 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -26572,9 +26572,7 @@ fn zirFuncFancy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A if (val.isGenericPoison()) { break :blk null; } - const alignment = try sema.validateAlignAllowZero(block, align_src, try val.toUnsignedIntSema(pt)); - const default = target_util.defaultFunctionAlignment(target); - break :blk if (alignment == default) .none else alignment; + break :blk try sema.validateAlignAllowZero(block, align_src, try val.toUnsignedIntSema(pt)); } else if (extra.data.bits.has_align_ref) blk: { const align_ref: Zir.Inst.Ref = @enumFromInt(sema.code.extra[extra_index]); extra_index += 1; @@ -26592,9 +26590,7 @@ fn zirFuncFancy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A error.GenericPoison => break :blk null, else => |e| return e, }; - const alignment = try sema.validateAlignAllowZero(block, align_src, try align_val.toUnsignedIntSema(pt)); - const default = target_util.defaultFunctionAlignment(target); - break :blk if (alignment == default) .none else alignment; + break :blk try sema.validateAlignAllowZero(block, align_src, try align_val.toUnsignedIntSema(pt)); } else .none; const @"addrspace": ?std.builtin.AddressSpace = if (extra.data.bits.has_addrspace_body) blk: { diff --git a/src/Type.zig b/src/Type.zig index c6feb9ddd4..582e997ac7 100644 --- a/src/Type.zig +++ b/src/Type.zig @@ -1020,7 +1020,7 @@ pub fn abiAlignmentInner( }, // represents machine code; not a pointer - .func_type => return .{ .scalar = target_util.defaultFunctionAlignment(target) }, + .func_type => return .{ .scalar = target_util.minFunctionAlignment(target) }, .simple_type => |t| switch (t) { .bool, diff --git a/src/target.zig b/src/target.zig index cce1787f33..87674cda89 100644 --- a/src/target.zig +++ b/src/target.zig @@ -459,7 +459,8 @@ pub fn llvmMachineAbi(target: std.Target) ?[:0]const u8 { } } -/// This function returns 1 if function alignment is not observable or settable. +/// This function returns 1 if function alignment is not observable or settable. Note that this +/// value will not necessarily match the backend's default function alignment (e.g. for LLVM). pub fn defaultFunctionAlignment(target: std.Target) Alignment { // Overrides of the minimum for performance. return switch (target.cpu.arch) { From 762ad4c6f49dfa1b8893353fb099d4e93672733f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Sun, 20 Oct 2024 08:52:32 +0200 Subject: [PATCH 4/6] link: Use defaultFunctionAlignment() when function alignment is unspecified. max(user_align, minFunctionAlignment()) is only appropriate when the user has actually given an explicit, non-zero alignment value. --- src/link/Coff.zig | 8 +++++--- src/link/Elf/ZigObject.zig | 8 +++++--- src/link/MachO/ZigObject.zig | 8 +++++--- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 1b25ee5e09..4da960f552 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -1391,9 +1391,11 @@ fn updateNavCode( log.debug("updateNavCode {} 0x{x}", .{ nav.fqn.fmt(ip), nav_index }); - const required_alignment = pt.navAlignment(nav_index).max( - target_util.minFunctionAlignment(zcu.navFileScope(nav_index).mod.resolved_target.result), - ); + const target = zcu.navFileScope(nav_index).mod.resolved_target.result; + const required_alignment = switch (pt.navAlignment(nav_index)) { + .none => target_util.defaultFunctionAlignment(target), + else => |a| a.maxStrict(target_util.minFunctionAlignment(target)), + }; const nav_metadata = self.navs.get(nav_index).?; const atom_index = nav_metadata.atom; diff --git a/src/link/Elf/ZigObject.zig b/src/link/Elf/ZigObject.zig index 893d3ce336..d77b7fc39b 100644 --- a/src/link/Elf/ZigObject.zig +++ b/src/link/Elf/ZigObject.zig @@ -1277,9 +1277,11 @@ fn updateNavCode( log.debug("updateNavCode {}({d})", .{ nav.fqn.fmt(ip), nav_index }); - const required_alignment = pt.navAlignment(nav_index).max( - target_util.minFunctionAlignment(zcu.navFileScope(nav_index).mod.resolved_target.result), - ); + const target = zcu.navFileScope(nav_index).mod.resolved_target.result; + const required_alignment = switch (pt.navAlignment(nav_index)) { + .none => target_util.defaultFunctionAlignment(target), + else => |a| a.maxStrict(target_util.minFunctionAlignment(target)), + }; const sym = self.symbol(sym_index); const esym = &self.symtab.items(.elf_sym)[sym.esym_index]; diff --git a/src/link/MachO/ZigObject.zig b/src/link/MachO/ZigObject.zig index 666a73f600..097be1bc70 100644 --- a/src/link/MachO/ZigObject.zig +++ b/src/link/MachO/ZigObject.zig @@ -962,9 +962,11 @@ fn updateNavCode( log.debug("updateNavCode {} 0x{x}", .{ nav.fqn.fmt(ip), nav_index }); - const required_alignment = pt.navAlignment(nav_index).max( - target_util.minFunctionAlignment(zcu.navFileScope(nav_index).mod.resolved_target.result), - ); + const target = zcu.navFileScope(nav_index).mod.resolved_target.result; + const required_alignment = switch (pt.navAlignment(nav_index)) { + .none => target_util.defaultFunctionAlignment(target), + else => |a| a.maxStrict(target_util.minFunctionAlignment(target)), + }; const sect = &macho_file.sections.items(.header)[sect_index]; const sym = &self.symbols.items[sym_index]; From 43344833c52dad441475c5eaceb257f524980c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Sun, 20 Oct 2024 09:09:01 +0200 Subject: [PATCH 5/6] link.Dwarf: Fix function alignment calculation to match the rest of the linker. In particular, for user-specified alignment values, we need to do max(user_align, minFunctionAlignment()) to respect the ABI minimum. --- src/link/Dwarf.zig | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/link/Dwarf.zig b/src/link/Dwarf.zig index 3b5e4fddeb..2c5876bd81 100644 --- a/src/link/Dwarf.zig +++ b/src/link/Dwarf.zig @@ -2372,8 +2372,11 @@ pub fn initWipNav(dwarf: *Dwarf, pt: Zcu.PerThread, nav_index: InternPool.Nav.In try wip_nav.infoAddrSym(sym_index, 0); wip_nav.func_high_pc = @intCast(wip_nav.debug_info.items.len); try diw.writeInt(u32, 0, dwarf.endian); - try uleb128(diw, nav.status.resolved.alignment.toByteUnits() orelse - target_info.defaultFunctionAlignment(file.mod.resolved_target.result).toByteUnits().?); + const target = file.mod.resolved_target.result; + try uleb128(diw, switch (nav.status.resolved.alignment) { + .none => target_info.defaultFunctionAlignment(target), + else => |a| a.maxStrict(target_info.minFunctionAlignment(target)), + }.toByteUnits().?); try diw.writeByte(@intFromBool(false)); try diw.writeByte(@intFromBool(func_type.return_type == .noreturn_type)); From 6c1e306484530bfeaf26450fb83a70ff1c323bcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20R=C3=B8nne=20Petersen?= Date: Tue, 15 Oct 2024 20:29:49 +0200 Subject: [PATCH 6/6] llvm: Add some missing fnptr alignment specifications in DataLayoutBuilder. --- src/codegen/llvm.zig | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index bd59cef569..12ebdf4128 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -458,10 +458,15 @@ const DataLayoutBuilder = struct { if (idx != size) try writer.print(":{d}", .{idx}); } } - if (self.target.cpu.arch.isArmOrThumb()) try writer.writeAll("-Fi8") // for thumb interwork + if (self.target.cpu.arch.isArmOrThumb()) + try writer.writeAll("-Fi8") // for thumb interwork else if (self.target.cpu.arch == .powerpc64 and - self.target.os.tag != .freebsd and self.target.abi != .musl) + self.target.os.tag != .freebsd and + self.target.os.tag != .openbsd and + !self.target.abi.isMusl()) try writer.writeAll("-Fi64") + else if (self.target.cpu.arch.isPowerPC() and self.target.os.tag == .aix) + try writer.writeAll(if (self.target.cpu.arch.isPowerPC64()) "-Fi64" else "-Fi32") else if (self.target.cpu.arch.isPowerPC()) try writer.writeAll("-Fn32"); if (self.target.cpu.arch != .hexagon) { @@ -573,6 +578,8 @@ const DataLayoutBuilder = struct { self.target.os.tag == .uefi or self.target.os.tag == .windows or self.target.cpu.arch == .riscv32) try writer.print("-S{d}", .{stack_abi}); + if (self.target.cpu.arch.isAARCH64()) + try writer.writeAll("-Fn32"); switch (self.target.cpu.arch) { .hexagon, .ve => { try self.typeAlignment(.vector, 32, 128, 128, true, writer);