From b51e8c39aeac1383be51da05e25b1d17f11ae6ab Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Thu, 31 Jan 2019 16:00:15 +0000 Subject: [PATCH] [ThinLTO] Rename COMDATs for COFF when promoting/renaming COMDAT leader Summary: COFF requires that COMDAT name match that of the leader. When we promote and rename an internal leader in ThinLTO due to an import, ensure we subsequently rename the associated COMDAT. Similar to D31963 which did this during ThinLTO module splitting. Fixes PR40414. Reviewers: pcc, inglorion Subscribers: mehdi_amini, dexonsmith, dmajor, llvm-commits Differential Revision: https://reviews.llvm.org/D57395 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@352763 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Transforms/Utils/FunctionImportUtils.h | 5 +++ lib/Transforms/Utils/FunctionImportUtils.cpp | 17 ++++++++++ .../FunctionImport/Inputs/comdat.ll | 10 ++++++ test/Transforms/FunctionImport/comdat.ll | 32 +++++++++++++++++++ 4 files changed, 64 insertions(+) create mode 100644 test/Transforms/FunctionImport/Inputs/comdat.ll create mode 100644 test/Transforms/FunctionImport/comdat.ll diff --git a/include/llvm/Transforms/Utils/FunctionImportUtils.h b/include/llvm/Transforms/Utils/FunctionImportUtils.h index ac69908d962..9c2a9ea531e 100644 --- a/include/llvm/Transforms/Utils/FunctionImportUtils.h +++ b/include/llvm/Transforms/Utils/FunctionImportUtils.h @@ -43,6 +43,11 @@ class FunctionImportGlobalProcessing { /// to promote any non-renamable values. SmallPtrSet Used; + /// Keep track of any COMDATs that require renaming (because COMDAT + /// leader was promoted and renamed). Maps from original COMDAT to one + /// with new name. + DenseMap RenamedComdats; + /// Check if we should promote the given local value to global scope. bool shouldPromoteLocalToGlobal(const GlobalValue *SGV); diff --git a/lib/Transforms/Utils/FunctionImportUtils.cpp b/lib/Transforms/Utils/FunctionImportUtils.cpp index a122b28e52e..84c360a17b5 100644 --- a/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -248,6 +248,7 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { bool DoPromote = false; if (GV.hasLocalLinkage() && ((DoPromote = shouldPromoteLocalToGlobal(&GV)) || isPerformingImport())) { + auto Name = GV.getName(); // Once we change the name or linkage it is difficult to determine // again whether we should promote since shouldPromoteLocalToGlobal needs // to locate the summary (based on GUID from name and linkage). Therefore, @@ -256,6 +257,12 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { GV.setLinkage(getLinkage(&GV, DoPromote)); if (!GV.hasLocalLinkage()) GV.setVisibility(GlobalValue::HiddenVisibility); + + // If we are renaming a COMDAT leader, ensure that we record the COMDAT + // for later renaming as well. This is required for COFF. + if (const auto *C = GV.getComdat()) + if (C->getName() == Name) + RenamedComdats.try_emplace(C, M.getOrInsertComdat(GV.getName())); } else GV.setLinkage(getLinkage(&GV, /* DoPromote */ false)); @@ -280,6 +287,16 @@ void FunctionImportGlobalProcessing::processGlobalsForThinLTO() { processGlobalForThinLTO(SF); for (GlobalAlias &GA : M.aliases()) processGlobalForThinLTO(GA); + + // Replace any COMDATS that required renaming (because the COMDAT leader was + // promoted and renamed). + if (!RenamedComdats.empty()) + for (auto &GO : M.global_objects()) + if (auto *C = GO.getComdat()) { + auto Replacement = RenamedComdats.find(C); + if (Replacement != RenamedComdats.end()) + GO.setComdat(Replacement->second); + } } bool FunctionImportGlobalProcessing::run() { diff --git a/test/Transforms/FunctionImport/Inputs/comdat.ll b/test/Transforms/FunctionImport/Inputs/comdat.ll new file mode 100644 index 00000000000..1df6f25351e --- /dev/null +++ b/test/Transforms/FunctionImport/Inputs/comdat.ll @@ -0,0 +1,10 @@ +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc19.0.24215" + +define void @main() { +entry: + call i8* @lwt_fun() + ret void +} + +declare i8* @lwt_fun() diff --git a/test/Transforms/FunctionImport/comdat.ll b/test/Transforms/FunctionImport/comdat.ll new file mode 100644 index 00000000000..29e8cb538ab --- /dev/null +++ b/test/Transforms/FunctionImport/comdat.ll @@ -0,0 +1,32 @@ +; Test to ensure that comdat is renamed consistently when comdat leader is +; promoted and renamed due to an import. Required by COFF. + +; REQUIRES: x86-registered-target + +; RUN: opt -thinlto-bc -o %t1.bc %s +; RUN: opt -thinlto-bc -o %t2.bc %S/Inputs/comdat.ll +; RUN: llvm-lto2 run -save-temps -o %t3 %t1.bc %t2.bc \ +; RUN: -r %t1.bc,lwt_fun,plx \ +; RUN: -r %t2.bc,main,plx \ +; RUN: -r %t2.bc,lwt_fun, +; RUN: llvm-dis -o - %t3.1.3.import.bc | FileCheck %s + +target datalayout = "e-m:w-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc19.0.24215" + +; CHECK: $lwt.llvm.[[HASH:[0-9]+]] = comdat any +$lwt = comdat any + +; CHECK: @lwt_aliasee = private unnamed_addr global {{.*}}, comdat($lwt.llvm.[[HASH]]) +@lwt_aliasee = private unnamed_addr global [1 x i8*] [i8* null], comdat($lwt) + +; CHECK: @lwt.llvm.[[HASH]] = hidden unnamed_addr alias +@lwt = internal unnamed_addr alias [1 x i8*], [1 x i8*]* @lwt_aliasee + +; Below function should get imported into other module, resulting in @lwt being +; promoted and renamed. +define i8* @lwt_fun() { + %1 = getelementptr inbounds [1 x i8*], [1 x i8*]* @lwt, i32 0, i32 0 + %2 = load i8*, i8** %1 + ret i8* %2 +} -- 2.40.0