[PATCH] D70128: [ThinLTO] Add correctness check for RO/WO variable import
Eugene Leviant via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 12 08:46:11 PST 2019
evgeny777 created this revision.
evgeny777 added a reviewer: tejohnson.
Herald added subscribers: dexonsmith, hiraditya, inglorion.
Herald added a project: LLVM.
This was suggested in D70006 <https://reviews.llvm.org/D70006>. The check is a bit expensive so is put under NDEBUG
https://reviews.llvm.org/D70128
Files:
llvm/lib/Transforms/IPO/FunctionImport.cpp
llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
Index: llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
===================================================================
--- llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
+++ llvm/lib/Transforms/Utils/FunctionImportUtils.cpp
@@ -250,12 +250,12 @@
// contains summaries from the source modules if they are being imported.
// 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()));
+ auto *GVS = dyn_cast_or_null<GlobalVarSummary>(
+ 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
+ // 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
Index: llvm/lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- llvm/lib/Transforms/IPO/FunctionImport.cpp
+++ llvm/lib/Transforms/IPO/FunctionImport.cpp
@@ -627,6 +627,35 @@
}
#endif
+static bool
+checkVariableImport(const ModuleSummaryIndex &Index,
+ StringMap<FunctionImporter::ImportMapTy> &ImportLists,
+ StringMap<FunctionImporter::ExportSetTy> &ExportLists) {
+
+ DenseSet<GlobalValue::GUID> ReadOrWriteOnlyVars;
+ auto IsReadOrWriteOnlyVar = [&](StringRef ModuleId, GlobalValue::GUID G) {
+ auto *GVS = dyn_cast_or_null<GlobalVarSummary>(
+ Index.findSummaryInModule(G, ModuleId));
+ return GVS && (Index.isReadOnly(GVS) || Index.isWriteOnly(GVS));
+ };
+
+ for (auto &ExportPerModule : ExportLists)
+ for (auto GUID : ExportPerModule.second)
+ if (IsReadOrWriteOnlyVar(ExportPerModule.first(), GUID))
+ ReadOrWriteOnlyVars.insert(GUID);
+
+ // Checks that all GUIDs of read/writeonly vars we see in export lists
+ // are also in the import lists. Otherwise we my face linker undefs,
+ // because readonly and writeonly vars are internalized in their
+ // source modules.
+ for (auto GUID : ReadOrWriteOnlyVars)
+ for (auto &ImportPerModule : ImportLists)
+ for (auto &ExportPerModule : ImportPerModule.second)
+ if (ExportPerModule.second.find(GUID) == ExportPerModule.second.end())
+ return false;
+ return true;
+}
+
/// Compute all the import and export for every module using the Index.
void llvm::ComputeCrossModuleImport(
const ModuleSummaryIndex &Index,
@@ -660,6 +689,7 @@
}
#ifndef NDEBUG
+ assert(checkVariableImport(Index, ImportLists, ExportLists));
LLVM_DEBUG(dbgs() << "Import/Export lists for " << ImportLists.size()
<< " modules:\n");
for (auto &ModuleImports : ImportLists) {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70128.228901.patch
Type: text/x-patch
Size: 3167 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191112/cbfa4690/attachment.bin>
More information about the llvm-commits
mailing list