diff options
author | Teresa Johnson <tejohnson@google.com> | 2023-05-01 19:04:00 -0700 |
---|---|---|
committer | Teresa Johnson <tejohnson@google.com> | 2023-05-02 07:49:03 -0700 |
commit | 48f18ecd8293e7828bf7ae2eeac975785892a0c4 (patch) | |
tree | 7be408cd67be495844d52c6b5606cde83da06bd1 /llvm | |
parent | 46740dd02babfc47edd9f8fdb03479ad61223246 (diff) | |
download | llvm-48f18ecd8293e7828bf7ae2eeac975785892a0c4.zip llvm-48f18ecd8293e7828bf7ae2eeac975785892a0c4.tar.gz llvm-48f18ecd8293e7828bf7ae2eeac975785892a0c4.tar.bz2 |
[ThinLTO] Loosen up variable importing correctness checks
After importing variables, we do some checking to ensure that variables
marked read or write only, which have been marked exported (e.g.
because a referencing function has been exported), are on at least one
module's imports list. This is because the read or write only variables
will be internalized, so we need a copy any any module that references
it.
This checking is overly conservative in the case of linkonce_odr or
other linkage types where there can already be a duplicate copy in
existence in the importing module, which therefore wouldn't need to
import it. Loosen up the checking for these linkage types.
Fixes https://github.com/llvm/llvm-project/issues/62468.
Differential Revision: https://reviews.llvm.org/D149630
Diffstat (limited to 'llvm')
-rw-r--r-- | llvm/lib/Transforms/IPO/FunctionImport.cpp | 14 | ||||
-rw-r--r-- | llvm/test/ThinLTO/X86/check_var_import_odr.ll | 41 |
2 files changed, 51 insertions, 4 deletions
diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 33da236..10718bb 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -678,17 +678,23 @@ checkVariableImport(const ModuleSummaryIndex &Index, // 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. - auto IsReadOrWriteOnlyVar = [&](StringRef ModulePath, const ValueInfo &VI) { + // source modules. The exception would be if it has a linkage type indicating + // that there may have been a copy existing in the importing module (e.g. + // linkonce_odr). In that case we cannot accurately do this checking. + auto IsReadOrWriteOnlyVarNeedingImporting = [&](StringRef ModulePath, + const ValueInfo &VI) { auto *GVS = dyn_cast_or_null<GlobalVarSummary>( Index.findSummaryInModule(VI, ModulePath)); - return GVS && (Index.isReadOnly(GVS) || Index.isWriteOnly(GVS)); + return GVS && (Index.isReadOnly(GVS) || Index.isWriteOnly(GVS)) && + !(GVS->linkage() == GlobalValue::AvailableExternallyLinkage || + GVS->linkage() == GlobalValue::WeakODRLinkage || + GVS->linkage() == GlobalValue::LinkOnceODRLinkage); }; for (auto &ExportPerModule : ExportLists) for (auto &VI : ExportPerModule.second) if (!FlattenedImports.count(VI.getGUID()) && - IsReadOrWriteOnlyVar(ExportPerModule.first(), VI)) + IsReadOrWriteOnlyVarNeedingImporting(ExportPerModule.first(), VI)) return false; return true; diff --git a/llvm/test/ThinLTO/X86/check_var_import_odr.ll b/llvm/test/ThinLTO/X86/check_var_import_odr.ll new file mode 100644 index 0000000..184bddc2 --- /dev/null +++ b/llvm/test/ThinLTO/X86/check_var_import_odr.ll @@ -0,0 +1,41 @@ +;; Test to ensure that we don't assert when checking variable importing +;; correctness for read only variables where the importing module contains +;; a linkonce_odr copy of the variable. In that case we do not need to +;; import the read only variable even when it has been exported from its +;; original module (in this case when we decided to import @bar). + +; RUN: split-file %s %t +; RUN: opt -module-summary %t/foo.ll -o %t/foo.o +; RUN: opt -module-summary %t/bar.ll -o %t/bar.o +; RUN: llvm-lto2 run %t/foo.o %t/bar.o -r=%t/foo.o,foo,pl -r=%t/foo.o,bar,l -r=%t/foo.o,qux,pl -r=%t/bar.o,bar,pl -r=%t/bar.o,qux, -o %t.out -save-temps +; RUN: llvm-dis %t.out.1.3.import.bc -o - | FileCheck %s + +;; Check that we have internalized @qux (since it is read only), and imported @bar. +; CHECK: @qux = internal global i32 0 +; CHECK: define available_externally hidden void @bar() + +;--- foo.ll + +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" + +@qux = linkonce_odr hidden global i32 0 + +define linkonce_odr hidden void @foo() { + call void @bar() + ret void +} + +declare hidden void @bar() + +;--- bar.ll + +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" + +@qux = linkonce_odr hidden global i32 0 + +define hidden void @bar() { + %1 = load i32, i32* @qux, align 8 + ret void +} |