From f215d98043ef948a996ac036609f4b71fa9c3c13 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 23 Sep 2021 23:39:58 -0700 Subject: [PATCH] stage2: LLVM backend: improved naming and exporting Introduce an explicit decl_map for *Decl to LLVMValueRef. Doc comment reproduced here: Ideally we would use `llvm_module.getNamedFunction` to go from *Decl to LLVM function, but that has some downsides: * we have to compute the fully qualified name every time we want to do the lookup * for externally linked functions, the name is not fully qualified, but when a Decl goes from exported to not exported and vice-versa, we would use the wrong version of the name and incorrectly get function not found in the llvm module. * it works for functions not all globals. Therefore, this table keeps track of the mapping. Non-exported functions now use fully-qualified symbol names. `Module.Decl.getFullyQualifiedName` now returns a sentinel-terminated slice which is useful to pass to LLVMAddFunction. Instead of using aliases for all external symbols, now the LLVM backend takes advantage of LLVMSetValueName to rename functions that become exported. Aliases are still used for the second and remaining exports. freeDecl is now handled properly in the LLVM backend, deleting the LLVMValueRef corresponding to the Decl being deleted. The linker backends for ELF, COFF, Mach-O, and Wasm had to be updated to forward the freeDecl call to the LLVM backend. --- src/Module.zig | 4 +- src/codegen/llvm.zig | 105 +++++++++++++++++++++++----------- src/codegen/llvm/bindings.zig | 9 +++ src/link/Coff.zig | 4 +- src/link/Elf.zig | 4 +- src/link/MachO.zig | 3 + src/link/Wasm.zig | 4 ++ src/stage1/codegen.cpp | 4 +- 8 files changed, 96 insertions(+), 41 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index a0e04dd478..a6a9225d75 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -619,11 +619,11 @@ pub const Decl = struct { return decl.namespace.renderFullyQualifiedName(unqualified_name, writer); } - pub fn getFullyQualifiedName(decl: Decl, gpa: *Allocator) ![]u8 { + pub fn getFullyQualifiedName(decl: Decl, gpa: *Allocator) ![:0]u8 { var buffer = std.ArrayList(u8).init(gpa); defer buffer.deinit(); try decl.renderFullyQualifiedName(buffer.writer()); - return buffer.toOwnedSlice(); + return buffer.toOwnedSliceSentinel(0); } pub fn typedValue(decl: Decl) error{AnalysisFail}!TypedValue { diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 4b7185d1ea..92524a09b7 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -155,6 +155,15 @@ pub const Object = struct { llvm_module: *const llvm.Module, context: *const llvm.Context, target_machine: *const llvm.TargetMachine, + /// Ideally we would use `llvm_module.getNamedFunction` to go from *Decl to LLVM function, + /// but that has some downsides: + /// * we have to compute the fully qualified name every time we want to do the lookup + /// * for externally linked functions, the name is not fully qualified, but when + /// a Decl goes from exported to not exported and vice-versa, we would use the wrong + /// version of the name and incorrectly get function not found in the llvm module. + /// * it works for functions not all globals. + /// Therefore, this table keeps track of the mapping. + decl_map: std.AutoHashMapUnmanaged(*const Module.Decl, *const llvm.Value), pub fn create(gpa: *Allocator, options: link.Options) !*Object { const obj = try gpa.create(Object); @@ -241,18 +250,20 @@ pub const Object = struct { .llvm_module = llvm_module, .context = context, .target_machine = target_machine, + .decl_map = .{}, }; } - pub fn deinit(self: *Object) void { + pub fn deinit(self: *Object, gpa: *Allocator) void { self.target_machine.dispose(); self.llvm_module.dispose(); self.context.dispose(); + self.decl_map.deinit(gpa); self.* = undefined; } pub fn destroy(self: *Object, gpa: *Allocator) void { - self.deinit(); + self.deinit(gpa); gpa.destroy(self); } @@ -450,41 +461,62 @@ pub const Object = struct { ) !void { // If the module does not already have the function, we ignore this function call // because we call `updateDeclExports` at the end of `updateFunc` and `updateDecl`. - const llvm_fn = self.llvm_module.getNamedFunction(decl.name) orelse return; + const llvm_fn = self.decl_map.get(decl) orelse return; const is_extern = decl.val.tag() == .extern_fn; - if (is_extern or exports.len != 0) { - llvm_fn.setLinkage(.External); + if (is_extern) { + llvm_fn.setValueName(decl.name); llvm_fn.setUnnamedAddr(.False); + llvm_fn.setLinkage(.External); + } else if (exports.len != 0) { + const exp_name = exports[0].options.name; + llvm_fn.setValueName2(exp_name.ptr, exp_name.len); + llvm_fn.setUnnamedAddr(.False); + switch (exports[0].options.linkage) { + .Internal => unreachable, + .Strong => llvm_fn.setLinkage(.External), + .Weak => llvm_fn.setLinkage(.WeakODR), + .LinkOnce => llvm_fn.setLinkage(.LinkOnceODR), + } + // If a Decl is exported more than one time (which is rare), + // we add aliases for all but the first export. + // TODO LLVM C API does not support deleting aliases. We need to + // patch it to support this or figure out how to wrap the C++ API ourselves. + // Until then we iterate over existing aliases and make them point + // to the correct decl, or otherwise add a new alias. Old aliases are leaked. + for (exports[1..]) |exp| { + const exp_name_z = try module.gpa.dupeZ(u8, exp.options.name); + defer module.gpa.free(exp_name_z); + + if (self.llvm_module.getNamedGlobalAlias(exp_name_z.ptr, exp_name_z.len)) |alias| { + alias.setAliasee(llvm_fn); + } else { + const alias = self.llvm_module.addAlias(llvm_fn.typeOf(), llvm_fn, exp_name_z); + switch (exp.options.linkage) { + .Internal => alias.setLinkage(.Internal), + .Strong => alias.setLinkage(.External), + .Weak => { + if (is_extern) { + alias.setLinkage(.ExternalWeak); + } else { + alias.setLinkage(.WeakODR); + } + }, + .LinkOnce => alias.setLinkage(.LinkOnceODR), + } + } + } } else { + const fqn = try decl.getFullyQualifiedName(module.gpa); + defer module.gpa.free(fqn); + llvm_fn.setValueName2(fqn.ptr, fqn.len); llvm_fn.setLinkage(.Internal); llvm_fn.setUnnamedAddr(.True); } - // TODO LLVM C API does not support deleting aliases. We need to - // patch it to support this or figure out how to wrap the C++ API ourselves. - // Until then we iterate over existing aliases and make them point - // to the correct decl, or otherwise add a new alias. Old aliases are leaked. - for (exports) |exp| { - const exp_name_z = try module.gpa.dupeZ(u8, exp.options.name); - defer module.gpa.free(exp_name_z); + } - if (self.llvm_module.getNamedGlobalAlias(exp_name_z.ptr, exp_name_z.len)) |alias| { - alias.setAliasee(llvm_fn); - } else { - const alias = self.llvm_module.addAlias(llvm_fn.typeOf(), llvm_fn, exp_name_z); - switch (exp.options.linkage) { - .Internal => alias.setLinkage(.Internal), - .Strong => alias.setLinkage(.External), - .Weak => { - if (is_extern) { - alias.setLinkage(.ExternalWeak); - } else { - alias.setLinkage(.WeakODR); - } - }, - .LinkOnce => alias.setLinkage(.LinkOnceODR), - } - } - } + pub fn freeDecl(self: *Object, decl: *Module.Decl) void { + const llvm_value = self.decl_map.get(decl) orelse return; + llvm_value.deleteGlobal(); } }; @@ -493,9 +525,8 @@ pub const DeclGen = struct { object: *Object, module: *Module, decl: *Module.Decl, - err_msg: ?*Module.ErrorMsg, - gpa: *Allocator, + err_msg: ?*Module.ErrorMsg, fn todo(self: *DeclGen, comptime format: []const u8, args: anytype) error{ OutOfMemory, CodegenFail } { @setCold(true); @@ -540,7 +571,8 @@ pub const DeclGen = struct { /// Note that this can be called before the function's semantic analysis has /// completed, so if any attributes rely on that, they must be done in updateFunc, not here. fn resolveLlvmFunction(self: *DeclGen, decl: *Module.Decl) !*const llvm.Value { - if (self.llvmModule().getNamedFunction(decl.name)) |llvm_fn| return llvm_fn; + const gop = try self.object.decl_map.getOrPut(self.gpa, decl); + if (gop.found_existing) return gop.value_ptr.*; assert(decl.has_tv); const zig_fn_type = decl.ty; @@ -570,7 +602,12 @@ pub const DeclGen = struct { .False, ); const llvm_addrspace = self.llvmAddressSpace(decl.@"addrspace"); - const llvm_fn = self.llvmModule().addFunctionInAddressSpace(decl.name, fn_type, llvm_addrspace); + + const fqn = try decl.getFullyQualifiedName(self.gpa); + defer self.gpa.free(fqn); + + const llvm_fn = self.llvmModule().addFunctionInAddressSpace(fqn, fn_type, llvm_addrspace); + gop.value_ptr.* = llvm_fn; const is_extern = decl.val.tag() == .extern_fn; if (!is_extern) { diff --git a/src/codegen/llvm/bindings.zig b/src/codegen/llvm/bindings.zig index be597949e2..c53ac08fdd 100644 --- a/src/codegen/llvm/bindings.zig +++ b/src/codegen/llvm/bindings.zig @@ -151,6 +151,15 @@ pub const Value = opaque { pub const setFunctionCallConv = LLVMSetFunctionCallConv; extern fn LLVMSetFunctionCallConv(Fn: *const Value, CC: CallConv) void; + + pub const setValueName = LLVMSetValueName; + extern fn LLVMSetValueName(Val: *const Value, Name: [*:0]const u8) void; + + pub const setValueName2 = LLVMSetValueName2; + extern fn LLVMSetValueName2(Val: *const Value, Name: [*]const u8, NameLen: usize) void; + + pub const deleteFunction = LLVMDeleteFunction; + extern fn LLVMDeleteFunction(Fn: *const Value) void; }; pub const Type = opaque { diff --git a/src/link/Coff.zig b/src/link/Coff.zig index 41b88881c4..a0e79eba20 100644 --- a/src/link/Coff.zig +++ b/src/link/Coff.zig @@ -770,7 +770,9 @@ fn finishUpdateDecl(self: *Coff, module: *Module, decl: *Module.Decl, code: []co } pub fn freeDecl(self: *Coff, decl: *Module.Decl) void { - if (self.llvm_object) |_| return; + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| return llvm_object.freeDecl(decl); + } // Appending to free lists is allowed to fail because the free lists are heuristics based anyway. self.freeTextBlock(&decl.link.coff); diff --git a/src/link/Elf.zig b/src/link/Elf.zig index 98eb0815a7..afa976d741 100644 --- a/src/link/Elf.zig +++ b/src/link/Elf.zig @@ -2147,7 +2147,9 @@ pub fn allocateDeclIndexes(self: *Elf, decl: *Module.Decl) !void { } pub fn freeDecl(self: *Elf, decl: *Module.Decl) void { - if (self.llvm_object) |_| return; + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| return llvm_object.freeDecl(decl); + } // Appending to free lists is allowed to fail because the free lists are heuristics based anyway. self.freeTextBlock(&decl.link.elf); diff --git a/src/link/MachO.zig b/src/link/MachO.zig index 93b3cc7e93..5551c31632 100644 --- a/src/link/MachO.zig +++ b/src/link/MachO.zig @@ -3501,6 +3501,9 @@ pub fn deleteExport(self: *MachO, exp: Export) void { } pub fn freeDecl(self: *MachO, decl: *Module.Decl) void { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| return llvm_object.freeDecl(decl); + } log.debug("freeDecl {*}", .{decl}); _ = self.decls.swapRemove(decl); // Appending to free lists is allowed to fail because the free lists are heuristics based anyway. diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index e3ec0bf255..2f5620e47a 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -339,6 +339,10 @@ pub fn updateDeclExports( } pub fn freeDecl(self: *Wasm, decl: *Module.Decl) void { + if (build_options.have_llvm) { + if (self.llvm_object) |llvm_object| return llvm_object.freeDecl(decl); + } + if (self.getFuncidx(decl)) |func_idx| { switch (decl.val.tag()) { .function => _ = self.funcs.swapRemove(func_idx), diff --git a/src/stage1/codegen.cpp b/src/stage1/codegen.cpp index 1304dcc004..f84847a9fe 100644 --- a/src/stage1/codegen.cpp +++ b/src/stage1/codegen.cpp @@ -487,9 +487,7 @@ static LLVMValueRef make_fn_llvm_value(CodeGen *g, ZigFn *fn) { if (mangled_symbol_buf) buf_destroy(mangled_symbol_buf); } } else { - if (llvm_fn == nullptr) { - llvm_fn = LLVMAddFunction(g->module, symbol_name, fn_llvm_type); - } + llvm_fn = LLVMAddFunction(g->module, symbol_name, fn_llvm_type); for (size_t i = 1; i < fn->export_list.length; i += 1) { GlobalExport *fn_export = &fn->export_list.items[i];