From 4075ccc7173ea9b48ef847378c007e29d275153d Mon Sep 17 00:00:00 2001 From: Bob Haarman Date: Wed, 12 Apr 2017 01:43:07 +0000 Subject: [PATCH] ThinLTOBitcodeWriter: keep comdats together, rename if leader is renamed Summary: COFF requires that every comdat contain a symbol with the same name as the comdat. ThinLTOBitcodeWriter renames symbols, which may cause this requirement to be violated. This change avoids such violations by renaming comdats if their leaders are renamed. It also keeps comdats together when splitting modules. Reviewers: pcc, mehdi_amini, tejohnson Reviewed By: pcc Subscribers: rnk, Prazek, llvm-commits Differential Revision: https://reviews.llvm.org/D31963 llvm-svn: 300019 --- llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp | 39 +++++++++-- .../test/Transforms/ThinLTOBitcodeWriter/comdat.ll | 80 ++++++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) create mode 100644 llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp index 393213f..65deb82 100644 --- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp @@ -69,15 +69,21 @@ std::string getModuleId(Module *M) { // 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) { + DenseMap RenamedComdats; 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) + RenamedComdats.try_emplace(C, ExportM.getOrInsertComdat(NewName)); ExportGV.setName(NewName); ExportGV.setLinkage(GlobalValue::ExternalLinkage); @@ -86,6 +92,14 @@ void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId) { ImportGV->setName(NewName); ImportGV->setVisibility(GlobalValue::HiddenVisibility); } + + if (!RenamedComdats.empty()) + for (auto &GO : ExportM.global_objects()) + if (auto *C = GO.getComdat()) { + auto Replacement = RenamedComdats.find(C); + if (Replacement != RenamedComdats.end()) + GO.setComdat(Replacement->second); + } } // Promote all internal (i.e. distinct) type ids used by the module by replacing @@ -273,8 +287,13 @@ void splitAndWriteThinLTOBitcode( // inline all implementations of the virtual function into each call site, // rather than using function attributes to perform local optimization. std::set EligibleVirtualFns; + // 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 (GlobalVariable &GV : M.globals()) - if (HasTypeMetadata(&GV)) + if (HasTypeMetadata(&GV)) { + if (const auto *C = GV.getComdat()) + MergedMComdats.insert(C); forEachVirtualFunction(GV.getInitializer(), [&](Function *F) { auto *RT = dyn_cast(F->getReturnType()); if (!RT || RT->getBitWidth() > 64 || F->arg_empty() || @@ -288,10 +307,14 @@ void splitAndWriteThinLTOBitcode( if (computeFunctionBodyMemoryAccess(*F, AARGetter(*F)) == MAK_ReadNone) EligibleVirtualFns.insert(F); }); + } ValueToValueMapTy VMap; std::unique_ptr MergedM( CloneModule(&M, VMap, [&](const GlobalValue *GV) -> bool { + if (const auto *C = GV->getComdat()) + if (MergedMComdats.count(C)) + return true; if (auto *F = dyn_cast(GV)) return EligibleVirtualFns.count(F); if (auto *GVar = dyn_cast_or_null(GV->getBaseObject())) @@ -309,11 +332,15 @@ void splitAndWriteThinLTOBitcode( 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 (const auto *C = GV->getComdat()) + if (MergedMComdats.count(C)) + return false; return true; }); diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll new file mode 100644 index 0000000..caea48e --- /dev/null +++ b/llvm/test/Transforms/ThinLTOBitcodeWriter/comdat.ll @@ -0,0 +1,80 @@ +; 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 + +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. +; MERGED: ${{"?lwt[^ ]+}} = comdat any +$lwt = comdat any + +; External comdat leader, type metadata on non-leader. All comdat +; members need to live in the merged module, internal members need to +; be renamed. +; MERGED: $nlwt = comdat any +$nlwt = comdat any + +; Comdat with two members without type metadata. All comdat members live in +; the ThinLTO module and no renaming needs to take place. +; THIN: $nt = comdat any +$nt = comdat any + +; MERGED: @lwt_aliasee = private unnamed_addr global +; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) +@lwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($lwt), !type !0 + +; MERGED: {{@"?lwt_nl[^ ]+}} = hidden unnamed_addr global +; MERGED-SAME: comdat(${{"?lwt[^ ]+}}) +; THIN: {{@"?lwt_nl[^ ]+}} = external hidden +@lwt_nl = internal unnamed_addr global i32 0, comdat($lwt) + +; MERGED: @nlwt_aliasee = private unnamed_addr global +; MERGED-SAME: comdat($nlwt) +@nlwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($nlwt), !type !0 + +; MERGED: @nlwt = unnamed_addr global +; MERGED-SAME: comdat +; THIN: @nlwt = external +@nlwt = unnamed_addr global i32 0, comdat + +; THIN: @nt = internal +; THIN-SAME: comdat +@nt = internal unnamed_addr global [1 x i8*] [i8* null], comdat + +; THIN: @nt_nl = internal +; THIN-SAME: comdat($nt) +@nt_nl = internal unnamed_addr global i32 0, comdat($nt) + +; MERGED: {{@"?lwt[^ ]+}} = hidden unnamed_addr alias +; THIN: {{@"?lwt[^ ]+}} = external hidden +@lwt = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee + +; MERGED: {{@"?nlwt_nl[^ ]+}} = hidden unnamed_addr alias +; THIN: {{@"?nlwt_nl[^ ]+}} = external hidden +@nlwt_nl = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @nlwt_aliasee + +; The functions below exist just to make sure the globals are used. +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 +} + +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 +} + +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@@"} -- 2.7.4