[llvm] r253796 - [ThinLTO] Handle bitcode without function summary sections gracefully
Teresa Johnson via llvm-commits
llvm-commits at lists.llvm.org
Sat Nov 21 13:55:49 PST 2015
Author: tejohnson
Date: Sat Nov 21 15:55:48 2015
New Revision: 253796
URL: http://llvm.org/viewvc/llvm-project?rev=253796&view=rev
Log:
[ThinLTO] Handle bitcode without function summary sections gracefully
Summary:
Several fixes to the handling of bitcode files without function summary
sections so that they are skipped during ThinLTO processing in llvm-lto
and the gold plugin when appropriate instead of aborting.
1 Don't assert when trying to add a FunctionInfo that doesn't have
a summary attached.
2 Skip FunctionInfo structures that don't have attached function summary
sections when trying to create the combined function summary.
3 In both llvm-lto and gold-plugin, check whether a bitcode file has
a function summary section before trying to parse the index, and skip
the bitcode file if it does not.
4 Fix hasFunctionSummaryInMemBuffer in BitcodeReader, which had a bug
where we returned to early while looking for the summary section.
Also added llvm-lto and gold-plugin based tests for cases where we
don't have function summaries in the bitcode file. I verified that
either the first couple fixes described above are enough to avoid the
crashes, or fixes 1,3,4. But have combined them all here for added
robustness.
Reviewers: joker.eph
Subscribers: llvm-commits, joker.eph
Differential Revision: http://reviews.llvm.org/D14903
Modified:
llvm/trunk/include/llvm/IR/FunctionInfo.h
llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/trunk/lib/IR/FunctionInfo.cpp
llvm/trunk/test/Linker/funcimport.ll
llvm/trunk/test/tools/gold/X86/thinlto.ll
llvm/trunk/tools/gold/gold-plugin.cpp
llvm/trunk/tools/llvm-lto/llvm-lto.cpp
Modified: llvm/trunk/include/llvm/IR/FunctionInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/FunctionInfo.h?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/FunctionInfo.h (original)
+++ llvm/trunk/include/llvm/IR/FunctionInfo.h Sat Nov 21 15:55:48 2015
@@ -196,8 +196,10 @@ public:
/// Add a function info for a function of the given name.
void addFunctionInfo(StringRef FuncName, std::unique_ptr<FunctionInfo> Info) {
- if (ExportingModule) {
- assert(Info->functionSummary());
+ // Update the HasExportedFunctions flag, but only if we had a function
+ // summary (i.e. we aren't parsing them lazily or have a bitcode file
+ // without a function summary section).
+ if (ExportingModule && Info->functionSummary()) {
if (ExportingModule->getModuleIdentifier() ==
Info->functionSummary()->modulePath())
HasExportedFunctions = true;
Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
+++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Sat Nov 21 15:55:48 2015
@@ -5566,12 +5566,14 @@ std::error_code FunctionIndexBitcodeRead
case BitstreamEntry::SubBlock:
if (CheckFuncSummaryPresenceOnly) {
- if (Entry.ID == bitc::FUNCTION_SUMMARY_BLOCK_ID)
+ if (Entry.ID == bitc::FUNCTION_SUMMARY_BLOCK_ID) {
SeenFuncSummary = true;
+ // No need to parse the rest since we found the summary.
+ return std::error_code();
+ }
if (Stream.SkipBlock())
return error("Invalid record");
- // No need to parse the rest since we found the summary.
- return std::error_code();
+ continue;
}
switch (Entry.ID) {
default: // Skip unknown content.
Modified: llvm/trunk/lib/IR/FunctionInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/FunctionInfo.cpp?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/lib/IR/FunctionInfo.cpp (original)
+++ llvm/trunk/lib/IR/FunctionInfo.cpp Sat Nov 21 15:55:48 2015
@@ -31,6 +31,10 @@ void FunctionInfoIndex::mergeFrom(std::u
assert(List.size() == 1);
std::unique_ptr<FunctionInfo> Info = std::move(List.front());
+ // Skip if there was no function summary section.
+ if (!Info->functionSummary())
+ continue;
+
// Add the module path string ref for this module if we haven't already
// saved a reference to it.
if (ModPath.empty())
Modified: llvm/trunk/test/Linker/funcimport.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Linker/funcimport.ll?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/test/Linker/funcimport.ll (original)
+++ llvm/trunk/test/Linker/funcimport.ll Sat Nov 21 15:55:48 2015
@@ -1,3 +1,10 @@
+; First ensure that the ThinLTO handling in llvm-link and llvm-lto handles
+; bitcode without function summary sections gracefully.
+; RUN: llvm-as %s -o %t.bc
+; RUN: llvm-as %p/Inputs/funcimport.ll -o %t2.bc
+; RUN: llvm-link %t.bc -functionindex=%t.bc -S
+; RUN: llvm-lto -thinlto -o %t3 %t.bc %t2.bc
+
; Do setup work for all below tests: generate bitcode and combined index
; RUN: llvm-as -function-summary %s -o %t.bc
; RUN: llvm-as -function-summary %p/Inputs/funcimport.ll -o %t2.bc
Modified: llvm/trunk/test/tools/gold/X86/thinlto.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/gold/X86/thinlto.ll?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/test/tools/gold/X86/thinlto.ll (original)
+++ llvm/trunk/test/tools/gold/X86/thinlto.ll Sat Nov 21 15:55:48 2015
@@ -1,3 +1,11 @@
+; First ensure that the ThinLTO handling in the gold plugin handles
+; bitcode without function summary sections gracefully.
+; RUN: llvm-as %s -o %t.o
+; RUN: llvm-as %p/Inputs/thinlto.ll -o %t2.o
+; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
+; RUN: --plugin-opt=thinlto \
+; RUN: -shared %t.o %t2.o -o %t3
+
; RUN: llvm-as -function-summary %s -o %t.o
; RUN: llvm-as -function-summary %p/Inputs/thinlto.ll -o %t2.o
Modified: llvm/trunk/tools/gold/gold-plugin.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/gold/gold-plugin.cpp?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/tools/gold/gold-plugin.cpp (original)
+++ llvm/trunk/tools/gold/gold-plugin.cpp Sat Nov 21 15:55:48 2015
@@ -620,6 +620,13 @@ getFunctionIndexForFile(claimed_file &F,
MemoryBufferRef BufferRef(StringRef((const char *)View, Info.filesize),
Info.name);
+
+ // Don't bother trying to build an index if there is no summary information
+ // in this bitcode file.
+ if (!object::FunctionIndexObjectFile::hasFunctionSummaryInMemBuffer(
+ BufferRef, diagnosticHandler))
+ return std::unique_ptr<FunctionInfoIndex>(nullptr);
+
ErrorOr<std::unique_ptr<object::FunctionIndexObjectFile>> ObjOrErr =
object::FunctionIndexObjectFile::create(BufferRef, diagnosticHandler);
@@ -911,6 +918,11 @@ static ld_plugin_status allSymbolsReadHo
std::unique_ptr<FunctionInfoIndex> Index =
getFunctionIndexForFile(F, File);
+
+ // Skip files without a function summary.
+ if (!Index)
+ continue;
+
CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId);
}
Modified: llvm/trunk/tools/llvm-lto/llvm-lto.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-lto/llvm-lto.cpp?rev=253796&r1=253795&r2=253796&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-lto/llvm-lto.cpp (original)
+++ llvm/trunk/tools/llvm-lto/llvm-lto.cpp Sat Nov 21 15:55:48 2015
@@ -192,24 +192,27 @@ static int listSymbols(StringRef Command
/// Parse the function index out of an IR file and return the function
/// index object if found, or nullptr if not.
-static std::unique_ptr<FunctionInfoIndex>
-getFunctionIndexForFile(StringRef Path, std::string &Error,
+static ErrorOr<std::unique_ptr<FunctionInfoIndex>>
+getFunctionIndexForFile(StringRef Path,
DiagnosticHandlerFunction DiagnosticHandler) {
std::unique_ptr<MemoryBuffer> Buffer;
ErrorOr<std::unique_ptr<MemoryBuffer>> BufferOrErr =
MemoryBuffer::getFile(Path);
- if (std::error_code EC = BufferOrErr.getError()) {
- Error = EC.message();
- return nullptr;
- }
+ if (std::error_code EC = BufferOrErr.getError())
+ return EC;
Buffer = std::move(BufferOrErr.get());
+
+ // Don't bother trying to build an index if there is no summary information
+ // in this bitcode file.
+ if (!object::FunctionIndexObjectFile::hasFunctionSummaryInMemBuffer(
+ Buffer->getMemBufferRef(), DiagnosticHandler))
+ return std::unique_ptr<FunctionInfoIndex>(nullptr);
+
ErrorOr<std::unique_ptr<object::FunctionIndexObjectFile>> ObjOrErr =
object::FunctionIndexObjectFile::create(Buffer->getMemBufferRef(),
DiagnosticHandler);
- if (std::error_code EC = ObjOrErr.getError()) {
- Error = EC.message();
- return nullptr;
- }
+ if (std::error_code EC = ObjOrErr.getError())
+ return EC;
return (*ObjOrErr)->takeIndex();
}
@@ -221,14 +224,18 @@ static int createCombinedFunctionIndex(S
FunctionInfoIndex CombinedIndex;
uint64_t NextModuleId = 0;
for (auto &Filename : InputFilenames) {
- std::string Error;
- std::unique_ptr<FunctionInfoIndex> Index =
- getFunctionIndexForFile(Filename, Error, diagnosticHandler);
- if (!Index) {
+ ErrorOr<std::unique_ptr<FunctionInfoIndex>> IndexOrErr =
+ getFunctionIndexForFile(Filename, diagnosticHandler);
+ if (std::error_code EC = IndexOrErr.getError()) {
+ std::string Error = EC.message();
errs() << Command << ": error loading file '" << Filename
<< "': " << Error << "\n";
return 1;
}
+ std::unique_ptr<FunctionInfoIndex> Index = std::move(IndexOrErr.get());
+ // Skip files without a function summary.
+ if (!Index)
+ continue;
CombinedIndex.mergeFrom(std::move(Index), ++NextModuleId);
}
std::error_code EC;
More information about the llvm-commits
mailing list