Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix MIPS PIC level and work around an LLVM bug for mips(el)-linux-gnueabi(hf) #21224

Merged
merged 3 commits into from
Aug 30, 2024
Merged

Fix MIPS PIC level and work around an LLVM bug for mips(el)-linux-gnueabi(hf) #21224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7d9edff
llvm: Set PIC level 1 for MIPS.
alexrp Aug 28, 2024
e585103
llvm: Disable FastISel on MIPS as a workaround for #21215.
alexrp Aug 28, 2024
a2e691d
Revert "add a workaround for miscompilation regarding alignment"
alexrp Aug 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 3 additions & 14 deletions lib/std/zig/Server.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,9 @@ pub fn receiveMessage(s: *Server) !InMessage.Header {
const buf = fifo.readableSlice(0);
assert(fifo.readableLength() == buf.len);
if (buf.len >= @sizeOf(Header)) {
// workaround for https://github.com/ziglang/zig/issues/14904
const bytes_len = bswap_and_workaround_u32(buf[4..][0..4]);
const tag = bswap_and_workaround_tag(buf[0..][0..4]);
const header: *align(1) const Header = @ptrCast(buf[0..@sizeOf(Header)]);
const bytes_len = bswap(header.bytes_len);
const tag = bswap(header.tag);

if (buf.len - @sizeOf(Header) >= bytes_len) {
fifo.discard(@sizeOf(Header));
Expand Down Expand Up @@ -307,17 +307,6 @@ fn bswap_u32_array(slice: []u32) void {
for (slice) |*elem| elem.* = @byteSwap(elem.*);
}

/// workaround for https://github.com/ziglang/zig/issues/14904
fn bswap_and_workaround_u32(bytes_ptr: *const [4]u8) u32 {
return std.mem.readInt(u32, bytes_ptr, .little);
}

/// workaround for https://github.com/ziglang/zig/issues/14904
fn bswap_and_workaround_tag(bytes_ptr: *const [4]u8) InMessage.Tag {
const int = std.mem.readInt(u32, bytes_ptr, .little);
return @as(InMessage.Tag, @enumFromInt(int));
}

const OutMessage = std.zig.Server.Message;
const InMessage = std.zig.Client.Message;

Expand Down
9 changes: 7 additions & 2 deletions src/codegen/llvm.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,8 +1259,11 @@ pub const Object = struct {
);
errdefer target_machine.dispose();

if (pic) module.setModulePICLevel();
if (comp.config.pie) module.setModulePIELevel();
const large_pic = target_util.usesLargePIC(comp.root_mod.resolved_target.result);

if (pic) module.setModulePICLevel(large_pic);
if (comp.config.pie) module.setModulePIELevel(large_pic);

if (code_model != .Default) module.setModuleCodeModel(code_model);

if (comp.llvm_opt_bisect_limit >= 0) {
Expand All @@ -1277,6 +1280,8 @@ pub const Object = struct {
.tsan = options.sanitize_thread,
.sancov = options.fuzz,
.lto = options.lto,
// https://github.com/ziglang/zig/issues/21215
.allow_fast_isel = !comp.root_mod.resolved_target.result.cpu.arch.isMIPS(),
.asm_filename = null,
.bin_filename = options.bin_path,
.llvm_ir_filename = options.post_ir_path,
Expand Down
5 changes: 3 additions & 2 deletions src/codegen/llvm/bindings.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ pub const Module = opaque {
extern fn LLVMDisposeModule(*Module) void;

pub const setModulePICLevel = ZigLLVMSetModulePICLevel;
extern fn ZigLLVMSetModulePICLevel(module: *Module) void;
extern fn ZigLLVMSetModulePICLevel(module: *Module, big: bool) void;

pub const setModulePIELevel = ZigLLVMSetModulePIELevel;
extern fn ZigLLVMSetModulePIELevel(module: *Module) void;
extern fn ZigLLVMSetModulePIELevel(module: *Module, large: bool) void;

pub const setModuleCodeModel = ZigLLVMSetModuleCodeModel;
extern fn ZigLLVMSetModuleCodeModel(module: *Module, code_model: CodeModel) void;
Comment on lines 55 to 62
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This API usage should not exist, we already have the ability to modify the module freely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, similar to float ABI, you can't set a code model on a bitcode module. So for these, we're stuck with the API (or llc later).

PIC/PIE are just module flags though, so I can look into emitting the module flags on our side in a follow-up PR if that's ok.

Copy link
Member

@jacobly0 jacobly0 Aug 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, similar to float ABI, you can't set a code model on a bitcode module.

Correct for float ABI, but not for code model:

!7 = !{i32 1, !"Code Model", i32 1}

Generally anything being set on llvm::Module is part of the module, anything just being passed to codegen is not.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #21238 so this isn't forgotten.

Expand Down Expand Up @@ -91,6 +91,7 @@ pub const TargetMachine = opaque {
tsan: bool,
sancov: bool,
lto: bool,
allow_fast_isel: bool,
asm_filename: ?[*:0]const u8,
bin_filename: ?[*:0]const u8,
llvm_ir_filename: ?[*:0]const u8,
Expand Down
6 changes: 6 additions & 0 deletions src/target.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ pub fn requiresPIC(target: std.Target, linking_libc: bool) bool {
(target.abi == .ohos and target.cpu.arch == .aarch64);
}

pub fn usesLargePIC(target: std.Target) bool {
// MIPS always uses PIC level 1; other platforms vary in their default PIC levels, but they
// support both level 1 and 2, in which case we prefer 2.
return !target.cpu.arch.isMIPS();
}

/// This is not whether the target supports Position Independent Code, but whether the -fPIC
/// C compiler argument is valid to Clang.
pub fn supports_fpic(target: std.Target) bool {
Expand Down
17 changes: 11 additions & 6 deletions src/zig_llvm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ static const bool assertions_on = false;

LLVMTargetMachineRef ZigLLVMCreateTargetMachine(LLVMTargetRef T, const char *Triple,
const char *CPU, const char *Features, LLVMCodeGenOptLevel Level, LLVMRelocMode Reloc,
LLVMCodeModel CodeModel, bool function_sections, bool data_sections, ZigLLVMABIType float_abi,
LLVMCodeModel CodeModel, bool function_sections, bool data_sections, ZigLLVMABIType float_abi,
const char *abi_name)
{
std::optional<Reloc::Model> RM;
Expand Down Expand Up @@ -258,7 +258,6 @@ ZIG_EXTERN_C bool ZigLLVMTargetMachineEmitToFile(LLVMTargetMachineRef targ_machi
options.bin_filename? options.bin_filename : options.asm_filename);

TargetMachine &target_machine = *reinterpret_cast<TargetMachine*>(targ_machine_ref);
target_machine.setO0WantsFastISel(true);

Module &llvm_module = *unwrap(module_ref);

Expand Down Expand Up @@ -369,6 +368,12 @@ ZIG_EXTERN_C bool ZigLLVMTargetMachineEmitToFile(LLVMTargetMachineRef targ_machi
}
}

if (options.allow_fast_isel) {
target_machine.setO0WantsFastISel(true);
} else {
target_machine.setFastISel(false);
}

// Optimization phase
module_pm.run(llvm_module, module_am);

Expand Down Expand Up @@ -430,12 +435,12 @@ void ZigLLVMParseCommandLineOptions(size_t argc, const char *const *argv) {
cl::ParseCommandLineOptions(argc, argv);
}

void ZigLLVMSetModulePICLevel(LLVMModuleRef module) {
unwrap(module)->setPICLevel(PICLevel::Level::BigPIC);
void ZigLLVMSetModulePICLevel(LLVMModuleRef module, bool big) {
unwrap(module)->setPICLevel(big ? PICLevel::Level::BigPIC : PICLevel::Level::SmallPIC);
}

void ZigLLVMSetModulePIELevel(LLVMModuleRef module) {
unwrap(module)->setPIELevel(PIELevel::Level::Large);
void ZigLLVMSetModulePIELevel(LLVMModuleRef module, bool large) {
unwrap(module)->setPIELevel(large ? PIELevel::Level::Large : PIELevel::Level::Small);
}

void ZigLLVMSetModuleCodeModel(LLVMModuleRef module, LLVMCodeModel code_model) {
Expand Down
7 changes: 4 additions & 3 deletions src/zig_llvm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ struct ZigLLVMEmitOptions {
bool tsan;
bool sancov;
bool lto;
bool allow_fast_isel;
const char *asm_filename;
const char *bin_filename;
const char *llvm_ir_filename;
Expand All @@ -77,7 +78,7 @@ enum ZigLLVMABIType {

ZIG_EXTERN_C LLVMTargetMachineRef ZigLLVMCreateTargetMachine(LLVMTargetRef T, const char *Triple,
const char *CPU, const char *Features, LLVMCodeGenOptLevel Level, LLVMRelocMode Reloc,
LLVMCodeModel CodeModel, bool function_sections, bool data_sections, enum ZigLLVMABIType float_abi,
LLVMCodeModel CodeModel, bool function_sections, bool data_sections, enum ZigLLVMABIType float_abi,
const char *abi_name);

ZIG_EXTERN_C void ZigLLVMSetOptBisectLimit(LLVMContextRef context_ref, int limit);
Expand Down Expand Up @@ -154,8 +155,8 @@ enum ZigLLVM_CallingConv {
ZigLLVM_MaxID = 1023,
};

ZIG_EXTERN_C void ZigLLVMSetModulePICLevel(LLVMModuleRef module);
ZIG_EXTERN_C void ZigLLVMSetModulePIELevel(LLVMModuleRef module);
ZIG_EXTERN_C void ZigLLVMSetModulePICLevel(LLVMModuleRef module, bool big);
ZIG_EXTERN_C void ZigLLVMSetModulePIELevel(LLVMModuleRef module, bool large);
ZIG_EXTERN_C void ZigLLVMSetModuleCodeModel(LLVMModuleRef module, LLVMCodeModel code_model);

ZIG_EXTERN_C void ZigLLVMParseCommandLineOptions(size_t argc, const char *const *argv);
Expand Down