[llvm] 7f92d66 - [ThinLTO] Fix bug when importing writeonly variables
via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 8 10:03:34 PST 2019
Author: evgeny
Date: 2019-11-08T20:50:34+03:00
New Revision: 7f92d66f378574ab2a02935b6614560ae9000539
URL: https://github.com/llvm/llvm-project/commit/7f92d66f378574ab2a02935b6614560ae9000539
DIFF: https://github.com/llvm/llvm-project/commit/7f92d66f378574ab2a02935b6614560ae9000539.diff
LOG: [ThinLTO] Fix bug when importing writeonly variables
Patch enables import of write-only variables with non-trivial initializers
to fix linker errors. Initializers of imported variables are converted to
'zeroinitializer' to avoid promotion of referenced objects.
Differential revision: https://reviews.llvm.org/D70006
Added:
llvm/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll
llvm/test/ThinLTO/X86/writeonly-with-refs.ll
Modified:
llvm/lib/IR/ModuleSummaryIndex.cpp
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
Removed:
################################################################################
diff --git a/llvm/lib/IR/ModuleSummaryIndex.cpp b/llvm/lib/IR/ModuleSummaryIndex.cpp
index d82d2e9cb733..b575b5b64428 100644
--- a/llvm/lib/IR/ModuleSummaryIndex.cpp
+++ b/llvm/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<GlobalVarSummary>(S->getBaseObject());
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp
index afc31bff3a60..83b03a07c457 100644
--- a/llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ b/llvm/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<GlobalVarSummary>(RefSummary.get())))
+ for (const auto &VI : RefSummary->refs())
+ for (const auto &RefFn : VI.getSummaryList())
+ MarkExported(VI, RefFn.get());
break;
}
}
diff --git a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp b/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
index dc9859156c40..401be6820439 100644
--- a/llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ b/llvm/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<GlobalVarSummary>(
- 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/llvm/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll b/llvm/test/ThinLTO/X86/Inputs/writeonly-with-refs.ll
new file mode 100644
index 000000000000..31ca2ad9fc55
--- /dev/null
+++ b/llvm/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* }
+
+ at _ZL3Obj = internal constant %struct.S { i32 42 }, align 4
+ at 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/llvm/test/ThinLTO/X86/writeonly-with-refs.ll b/llvm/test/ThinLTO/X86/writeonly-with-refs.ll
new file mode 100644
index 000000000000..63f75762c39b
--- /dev/null
+++ b/llvm/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
More information about the llvm-commits
mailing list