LLVM: handle extern function name collisions

Zig allows multiple extern functions with the same name, and the
backends have to handle this possibility.

For LLVM, we keep a sparse map of collisions, and then resolve them in
flushModule(). This introduces some technical debt that will have to be
resolved when adding incremental compilation support to the LLVM
backend.

2 files changed, 54 insertions, 12 deletions

diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig
index 9e9dd2cb0c..f7a1d732b4 100644
--- a/src/codegen/llvm.zig
+++ b/src/codegen/llvm.zig
@@ -214,6 +214,10 @@ pub const Object = struct {
     /// Note that the values are not added until flushModule, when all errors in
     /// the compilation are known.
     error_name_table: ?*const llvm.Value,
+    /// This map is usually very close to empty. It tracks only the cases when a
+    /// second extern Decl could not be emitted with the correct name due to a
+    /// name collision.
+    extern_collisions: std.AutoArrayHashMapUnmanaged(Module.Decl.Index, void),
 
     pub const TypeMap = std.HashMapUnmanaged(
         Type,
@@ -376,6 +380,7 @@ pub const Object = struct {
             .type_map_arena = std.heap.ArenaAllocator.init(gpa),
             .di_type_map = .{},
             .error_name_table = null,
+            .extern_collisions = .{},
         };
     }
 
@@ -392,6 +397,7 @@ pub const Object = struct {
         self.decl_map.deinit(gpa);
         self.type_map.deinit(gpa);
         self.type_map_arena.deinit();
+        self.extern_collisions.deinit(gpa);
         self.* = undefined;
     }
 
@@ -508,6 +514,22 @@ pub const Object = struct {
     fn resolveExportExternCollisions(object: *Object) !void {
         const mod = object.module;
 
+        // This map has externs with incorrect symbol names.
+        for (object.extern_collisions.keys()) |decl_index| {
+            const entry = object.decl_map.getEntry(decl_index) orelse continue;
+            const llvm_global = entry.value_ptr.*;
+            // Same logic as below but for externs instead of exports.
+            const decl = mod.declPtr(decl_index);
+            const other_global = object.getLlvmGlobal(decl.name) orelse continue;
+            if (other_global == llvm_global) continue;
+
+            const new_global_ptr = other_global.constBitCast(llvm_global.typeOf());
+            llvm_global.replaceAllUsesWith(new_global_ptr);
+            object.deleteLlvmGlobal(llvm_global);
+            entry.value_ptr.* = new_global_ptr;
+        }
+        object.extern_collisions.clearRetainingCapacity();
+
         const export_keys = mod.decl_exports.keys();
         for (mod.decl_exports.values()) |export_list, i| {
             const decl_index = export_keys[i];
@@ -997,6 +1019,15 @@ pub const Object = struct {
         return null;
     }
 
+    /// TODO can this be done with simpler logic / different API binding?
+    fn deleteLlvmGlobal(o: Object, llvm_global: *const llvm.Value) void {
+        if (o.llvm_module.getNamedFunction(llvm_global.getValueName()) != null) {
+            llvm_global.deleteFunction();
+            return;
+        }
+        return llvm_global.deleteGlobal();
+    }
+
     pub fn updateDeclExports(
         self: *Object,
         module: *Module,
@@ -1009,6 +1040,12 @@ pub const Object = struct {
         const decl = module.declPtr(decl_index);
         if (decl.isExtern()) {
             llvm_global.setValueName(decl.name);
+            if (self.getLlvmGlobal(decl.name)) |other_global| {
+                if (other_global != llvm_global) {
+                    log.debug("updateDeclExports isExtern()=true setValueName({s}) conflict", .{decl.name});
+                    try self.extern_collisions.put(module.gpa, decl_index, {});
+                }
+            }
             llvm_global.setUnnamedAddr(.False);
             llvm_global.setLinkage(.External);
             if (self.di_map.get(decl)) |di_node| {
@@ -2143,11 +2180,8 @@ pub const DeclGen = struct {
         log.debug("gen: {s} type: {}, value: {}", .{
             decl.name, decl.ty.fmtDebug(), decl.val.fmtDebug(),
         });
-
-        if (decl.val.castTag(.function)) |func_payload| {
-            _ = func_payload;
-            @panic("TODO llvm backend genDecl function pointer");
-        } else if (decl.val.castTag(.extern_fn)) |extern_fn| {
+        assert(decl.val.tag() != .function);
+        if (decl.val.castTag(.extern_fn)) |extern_fn| {
             _ = try dg.resolveLlvmFunction(extern_fn.data.owner_decl);
         } else {
             const target = dg.module.getTarget();
@@ -2246,12 +2280,14 @@ pub const DeclGen = struct {
         if (!is_extern) {
             llvm_fn.setLinkage(.Internal);
             llvm_fn.setUnnamedAddr(.True);
-        } else if (dg.module.getTarget().isWasm()) {
-            dg.addFnAttrString(llvm_fn, "wasm-import-name", std.mem.sliceTo(decl.name, 0));
-            if (decl.getExternFn().?.lib_name) |lib_name| {
-                const module_name = std.mem.sliceTo(lib_name, 0);
-                if (!std.mem.eql(u8, module_name, "c")) {
-                    dg.addFnAttrString(llvm_fn, "wasm-import-module", module_name);
+        } else {
+            if (dg.module.getTarget().isWasm()) {
+                dg.addFnAttrString(llvm_fn, "wasm-import-name", std.mem.sliceTo(decl.name, 0));
+                if (decl.getExternFn().?.lib_name) |lib_name| {
+                    const module_name = std.mem.sliceTo(lib_name, 0);
+                    if (!std.mem.eql(u8, module_name, "c")) {
+                        dg.addFnAttrString(llvm_fn, "wasm-import-module", module_name);
+                    }
                 }
             }
         }
diff --git a/test/behavior/bugs/529.zig b/test/behavior/bugs/529.zig
index e23321cd5c..b111cea564 100644
--- a/test/behavior/bugs/529.zig
+++ b/test/behavior/bugs/529.zig
@@ -8,8 +8,14 @@ comptime {
     _ = @import("529_other_file_2.zig");
 }
 
+const builtin = @import("builtin");
+
 test "issue 529 fixed" {
-    if (@import("builtin").zig_backend != .stage1) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
+    if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
 
     @import("529_other_file.zig").issue529(null);
     issue529(null);