diff --git a/lib/IR/ModuleSummaryIndex.cpp b/lib/IR/ModuleSummaryIndex.cpp index d82d2e9cb73..b575b5b6442 100644 --- a/lib/IR/ModuleSummaryIndex.cpp +++ b/lib/IR/ModuleSummaryIndex.cpp @@ -197,7 +197,21 @@ void ModuleSummaryIndex::propagateAttributes( bool ModuleSummaryIndex::canImportGlobalVar(GlobalValueSummary *S, bool AnalyzeRefs) const { auto HasRefsPreventingImport = [this](const GlobalVarSummary *GVS) { - return !isReadOnly(GVS) && GVS->refs().size(); + // We don't analyze GV references during attribute propagation, so + // GV with non-trivial initializer can be marked either read or + // write-only. + // Importing definiton of readonly GV with non-trivial initializer + // allows us doing some extra optimizations (like converting indirect + // calls to direct). + // Definition of writeonly GV with non-trivial initializer should also + // be imported. Not doing so will result in: + // a) GV internalization in source module (because it's writeonly) + // b) Importing of GV declaration to destination module as a result + // of promotion. + // c) Link error (external declaration with internal definition). + // However we do not promote objects referenced by writeonly GV + // initializer by means of converting it to 'zeroinitializer' + return !isReadOnly(GVS) && !isWriteOnly(GVS) && GVS->refs().size(); }; auto *GVS = cast(S->getBaseObject()); diff --git a/lib/Transforms/IPO/FunctionImport.cpp b/lib/Transforms/IPO/FunctionImport.cpp index afc31bff3a6..83b03a07c45 100644 --- a/lib/Transforms/IPO/FunctionImport.cpp +++ b/lib/Transforms/IPO/FunctionImport.cpp @@ -318,10 +318,14 @@ static void computeImportForReferencedGlobals( if (ILI.second) NumImportedGlobalVarsThinLink++; MarkExported(VI, RefSummary.get()); - // Promote referenced functions and variables - for (const auto &VI : RefSummary->refs()) - for (const auto &RefFn : VI.getSummaryList()) - MarkExported(VI, RefFn.get()); + // Promote referenced functions and variables. We don't promote + // objects referenced by writeonly variable initializer, because + // we convert such variables initializers to "zeroinitializer". + // See processGlobalForThinLTO. + if (!Index.isWriteOnly(cast(RefSummary.get()))) + for (const auto &VI : RefSummary->refs()) + for (const auto &RefFn : VI.getSummaryList()) + MarkExported(VI, RefFn.get()); break; } } diff --git a/lib/Transforms/Utils/FunctionImportUtils.cpp b/lib/Transforms/Utils/FunctionImportUtils.cpp index dc9859156c4..401be682043 100644 --- a/lib/Transforms/Utils/FunctionImportUtils.cpp +++ b/lib/Transforms/Utils/FunctionImportUtils.cpp @@ -12,6 +12,7 @@ //===----------------------------------------------------------------------===// #include "llvm/Transforms/Utils/FunctionImportUtils.h" +#include "llvm/IR/Constants.h" #include "llvm/IR/InstIterator.h" using namespace llvm; @@ -250,10 +251,20 @@ void FunctionImportGlobalProcessing::processGlobalForThinLTO(GlobalValue &GV) { // We might have a non-null VI and get here even in that case if the name // matches one in this module (e.g. weak or appending linkage). auto* GVS = dyn_cast_or_null( - ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier())); - // At this stage "maybe" is "definitely" - if (GVS && (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS))) + ImportIndex.findSummaryInModule(VI, M.getModuleIdentifier())); + if (GVS && + (ImportIndex.isReadOnly(GVS) || ImportIndex.isWriteOnly(GVS))) { V->addAttribute("thinlto-internalize"); + // Objects referenced by writeonly GV initializer should not be + // promoted, because there is no any kind of read access to them + // on behalf of this writeonly GV. To avoid promotion we convert + // GV initializer to 'zeroinitializer'. This effectively drops + // references in IR module (not in combined index), so we can + // ignore them when computing import. We do not export references + // of writeonly object. See computeImportForReferencedGlobals + if (ImportIndex.isWriteOnly(GVS) && GVS->refs().size()) + V->setInitializer(Constant::getNullValue(V->getValueType())); + } } } diff --git a/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll b/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll new file mode 100644 index 00000000000..31ca2ad9fc5 --- /dev/null +++ b/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll @@ -0,0 +1,17 @@ +; ModuleID = 'foo.o' +source_filename = "foo.cpp" +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +%struct.S = type { i32 } +%struct.Q = type { %struct.S* } + +@_ZL3Obj = internal constant %struct.S { i32 42 }, align 4 +@outer = dso_local local_unnamed_addr global %struct.Q { %struct.S* @_ZL3Obj }, align 8 + +; Function Attrs: nofree norecurse nounwind uwtable writeonly +define dso_local void @_Z3foov() local_unnamed_addr { +entry: + store %struct.S* null, %struct.S** getelementptr inbounds (%struct.Q, %struct.Q* @outer, i64 0, i32 0), align 8 + ret void +} diff --git a/test/ThinLTO/X86/writeonly-with-refs.ll b/test/ThinLTO/X86/writeonly-with-refs.ll new file mode 100644 index 00000000000..63f75762c39 --- /dev/null +++ b/test/ThinLTO/X86/writeonly-with-refs.ll @@ -0,0 +1,26 @@ +; RUN: opt -thinlto-bc %s -o %t1 +; RUN: opt -thinlto-bc %p/Inputs/writeonly-with-refs.ll -o %t2 +; RUN: llvm-lto2 run -save-temps %t1 %t2 -o %t-out \ +; RUN: -r=%t1,main,plx \ +; RUN: -r=%t1,_Z3foov,l \ +; RUN: -r=%t2,_Z3foov,pl \ +; RUN: -r=%t2,outer,pl + +; @outer should have been internalized and converted to zeroinitilizer. +; RUN: llvm-dis %t-out.1.5.precodegen.bc -o - | FileCheck %s +; RUN: llvm-dis %t-out.2.5.precodegen.bc -o - | FileCheck %s + +; CHECK: @outer = internal unnamed_addr global %struct.Q zeroinitializer + +source_filename = "main.cpp" +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +; Function Attrs: norecurse uwtable +define dso_local i32 @main() local_unnamed_addr { +entry: + tail call void @_Z3foov() + ret i32 0 +} + +declare dso_local void @_Z3foov() local_unnamed_addr