Index: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp =================================================================== --- lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp @@ -30,6 +30,7 @@ #include "llvm/Transforms/IPO.h" #include "llvm/Transforms/IPO/FunctionAttrs.h" #include "llvm/Transforms/Utils/Cloning.h" +#include using namespace llvm; namespace { @@ -68,16 +69,23 @@ // Promote each local-linkage entity defined by ExportM and used by ImportM by // changing visibility and appending the given ModuleId. -void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId) { +void promoteInternals( + Module &ExportM, Module &ImportM, StringRef ModuleId, + std::unordered_map &ComdatsToRename) { for (auto &ExportGV : ExportM.global_values()) { if (!ExportGV.hasLocalLinkage()) continue; - GlobalValue *ImportGV = ImportM.getNamedValue(ExportGV.getName()); + auto Name = ExportGV.getName(); + GlobalValue *ImportGV = ImportM.getNamedValue(Name); if (!ImportGV || ImportGV->use_empty()) continue; - std::string NewName = (ExportGV.getName() + ModuleId).str(); + std::string NewName = (Name + ModuleId).str(); + + if (const auto *C = ExportGV.getComdat()) + if (C->getName() == Name) + ComdatsToRename.emplace(Name, NewName); ExportGV.setName(NewName); ExportGV.setLinkage(GlobalValue::ExternalLinkage); @@ -231,6 +239,25 @@ forEachVirtualFunction(cast(Op), Fn); } +void renameComdats( + Module &M, + const std::unordered_map &ComdatsToRename) { + DenseMap NewComdats; + for (const auto &I : ComdatsToRename) + NewComdats.try_emplace(I.first, M.getOrInsertComdat(I.second)); + + for (auto &GO : M.global_objects()) + if (auto *C = GO.getComdat()) { + const auto I = NewComdats.find(C->getName()); + if (I != NewComdats.end()) + GO.setComdat(I->getSecond()); + } + + auto &ComdatSymTab = M.getComdatSymbolTable(); + for (const auto &I : ComdatsToRename) + ComdatSymTab.erase(I.first); +} + // If it's possible to split M into regular and thin LTO parts, do so and write // a multi-module bitcode file with the two parts to OS. Otherwise, write only a // regular LTO bitcode file to OS. @@ -289,14 +316,29 @@ EligibleVirtualFns.insert(F); }); + auto ShouldLiveInMergedM = [&](const GlobalValue *GV) -> bool { + if (auto *F = dyn_cast(GV)) + return EligibleVirtualFns.count(F); + if (auto *GVar = dyn_cast_or_null(GV->getBaseObject())) + return HasTypeMetadata(GVar); + return false; + }; + + // If any member of a comdat lives in MergedM, put all members of that + // comdat in MergedM to keep the comdat together. + DenseSet MergedMComdats; + for (const auto &GO : M.global_objects()) + if (const auto *C = GO.getComdat()) + if (ShouldLiveInMergedM(&GO)) + MergedMComdats.insert(C->getName()); + ValueToValueMapTy VMap; std::unique_ptr MergedM( CloneModule(&M, VMap, [&](const GlobalValue *GV) -> bool { - if (auto *F = dyn_cast(GV)) - return EligibleVirtualFns.count(F); - if (auto *GVar = dyn_cast_or_null(GV->getBaseObject())) - return HasTypeMetadata(GVar); - return false; + if (auto *C = GV->getComdat()) + if (MergedMComdats.count(C->getName())) + return true; + return ShouldLiveInMergedM(GV); })); StripDebugInfo(*MergedM); @@ -309,16 +351,27 @@ F.setComdat(nullptr); } - // Remove all globals with type metadata, as well as aliases pointing to them, - // from the thin LTO module. + // Remove all globals with type metadata, globals with comdats that live in + // MergedM, and aliases pointing to such globals from the thin LTO module. filterModule(&M, [&](const GlobalValue *GV) { if (auto *GVar = dyn_cast_or_null(GV->getBaseObject())) - return !HasTypeMetadata(GVar); + if (HasTypeMetadata(GVar)) + return false; + if (auto *C = GV->getComdat()) + if (MergedMComdats.count(C->getName())) + return false; return true; }); - promoteInternals(*MergedM, M, ModuleId); - promoteInternals(M, *MergedM, ModuleId); + std::unordered_map ComdatsToRename; + promoteInternals(*MergedM, M, ModuleId, ComdatsToRename); + promoteInternals(M, *MergedM, ModuleId, ComdatsToRename); + + // COFF requires that every comdat contain a symbol with the same name + // as the comdat. If we renamed any of those symbols, rename the comdat + // to match the new name. + renameComdats(*MergedM, ComdatsToRename); + renameComdats(M, ComdatsToRename); simplifyExternals(*MergedM); Index: test/Transforms/ThinLTOBitcodeWriter/comdat.ll =================================================================== --- /dev/null +++ test/Transforms/ThinLTOBitcodeWriter/comdat.ll @@ -0,0 +1,81 @@ +; RUN: opt -thinlto-bc -o %t %s +; RUN: llvm-modextract -n 0 -o - %t | llvm-dis | FileCheck --check-prefix=THIN %s +; RUN: llvm-modextract -n 1 -o - %t | llvm-dis | FileCheck --check-prefix=MERGED %s + +; MERGED: ${{"?lwt[^ ]+}} = comdat any +; MERGED: $nlwt = comdat any +; MERGED: {{@"?lwt_aliasee[^ ]*}} = private unnamed_addr global +; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) +; MERGED: {{@"?lwt_nl[^ ]+}} = hidden unnamed_addr global +; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) +; MERGED: {{@"?nlwt_aliasee[^ ]*}} = private unnamed_addr global +; MERGED-SAME: comdat($nlwt) +; MERGED: @nlwt = unnamed_addr global +; MERGED-SAME: comdat +; MERGED: {{@"?lwt[^ ]+}} = hidden unnamed_addr alias +; MERGED: {{@"?nlwt_nl[^ ]+}} = hidden unnamed_addr alias + +; THIN: $nt = comdat any +; THIN: {{@"?lwt_nl[^ ]+}} = external hidden +; THIN: @nlwt = external +; THIN: @nt = internal +; THIN-SAME: comdat +; THIN: @nt_nl = internal +; THIN-SAME: comdat($nt) +; THIN: {{@"?lwt[^ ]+}} = external hidden +; THIN: {{@"?nlwt_nl[^ ]+}} = external hidden + +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc19.0.24215" + +; Internal comdat leader with type metadata. All comdat members need to live +; in the merged module, and the comdat needs to be renamed. +$lwt = comdat any + +@lwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($lwt), !type !0 + +@lwt = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee + +@lwt_nl = internal unnamed_addr global i32 0, comdat($lwt) + +define i8* @lwt_fun() { + %1 = load i32, i32* @lwt_nl + %2 = getelementptr inbounds [1 x i8*], [1 x i8*]* @lwt, i32 0, i32 %1 + %3 = load i8*, i8** %2 + ret i8* %3 +} + +; External comdat leader, type metadata on non-leader. All comdat +; members need to live in the merged module, internal members need to +; be renamed. +$nlwt = comdat any + +@nlwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($nlwt), !type !0 + +@nlwt_nl = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee + +@nlwt = unnamed_addr global i32 0, comdat + +define i8* @nlwt_fun() { + %1 = load i32, i32* @nlwt + %2 = getelementptr inbounds [1 x i8*], [1 x i8*]* @nlwt_nl, i32 0, i32 %1 + %3 = load i8*, i8** %2 + ret i8* %3 +} + +; Comdat with two members without type metadata. All comdat members live in +; the ThinLTO module and no renaming needs to take place. +$nt = comdat any + +@nt = internal unnamed_addr global [1 x i8*] [i8* null], comdat + +@nt_nl = internal unnamed_addr global i32 0, comdat($nt) + +define i8* @nt_fun() { + %1 = load i32, i32* @nt_nl + %2 = getelementptr inbounds [1 x i8*], [1 x i8*]* @nt, i32 0, i32 %1 + %3 = load i8*, i8** %2 + ret i8* %3 +} + +!0 = !{i64 8, !"?AVA@@"}