This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Optionally ignore empty index file
ClosedPublic

Authored by tejohnson on Jan 5 2017, 9:26 AM.

Details

Summary

In order to simplify distributed build system integration, where actions
may be scheduled before the Thin Link which determines the list of
objects selected by the linker. The gold plugin currently will emit
0-sized index files for objects not selected by the link, to enable
checking for expected output files by the build system. If the build
system then schedules a backend action for these bitcode files, we want
to be able to fall back to normal compilation instead of failing.

Fallback is enabled under an option in LLVM (D28410), in which case a nullptr
is returned from llvm::getModuleSummaryIndexForFile. Clang can just proceed
with non-ThinLTO compilation in that case.

I am investigating whether this can be addressed in our build system, but that
is a longer term fix and so this enables a workaround in the meantime.

Event Timeline

tejohnson updated this revision to Diff 83266.Jan 5 2017, 9:26 AM
tejohnson retitled this revision from to [ThinLTO] Optionally ignore empty index file.
tejohnson updated this object.
tejohnson added a reviewer: mehdi_amini.
tejohnson added a subscriber: cfe-commits.
mehdi_amini added inline comments.Jan 5 2017, 10:39 AM
lib/CodeGen/BackendUtil.cpp
65

Is it common to do this in clang?

tejohnson added inline comments.Jan 5 2017, 10:42 AM
lib/CodeGen/BackendUtil.cpp
65

Doesn't look common, but I see one other LLVM internal option (in CodeGenPGO.cpp), and a few other uses in tests. Is there a better way to do internal options in clang?

tejohnson added inline comments.Jan 6 2017, 12:52 PM
lib/CodeGen/BackendUtil.cpp
65

Since this doesn't look like something commonly done in clang, I am splitting this into 2 patches, one in LLVM and one here. LLVM will instead contain the option, and return a nullptr from llvm::getModuleSummaryIndexForFile when the option is enabled and the file is empty. The clang side can just proceed to normal compilation if a nullptr CombinedIndex is returned (if the option is not enabled, LLVM would return an error instead).

tejohnson updated this revision to Diff 83405.Jan 6 2017, 12:52 PM

Move option to LLVM.

tejohnson updated this object.Jan 6 2017, 12:56 PM
mehdi_amini accepted this revision.Jan 6 2017, 12:58 PM
mehdi_amini edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jan 6 2017, 12:58 PM

LGTM

Thanks, let me know if the llvm companion patch (D28410) is ok as well.

mehdi_amini added a comment.EditedJan 6 2017, 1:33 PM

In order to simplify distributed build system integration, where actions
may be scheduled before the Thin Link which determines the list of
objects selected by the linker. The gold plugin currently will emit
0-sized index files for objects not selected by the link, to enable
checking for expected output files by the build system. If the build
system then schedules a backend action for these bitcode files, we want
to be able to fall back to normal compilation instead of failing.

Just thought about it: why not emit an empty *index* instead of an empty file?

In order to simplify distributed build system integration, where actions
may be scheduled before the Thin Link which determines the list of
objects selected by the linker. The gold plugin currently will emit
0-sized index files for objects not selected by the link, to enable
checking for expected output files by the build system. If the build
system then schedules a backend action for these bitcode files, we want
to be able to fall back to normal compilation instead of failing.

Just thought about it: why not emit an empty *index* instead of an empty file?

I thought about that initially. But it will also need some special handling as e.g. FunctionImportGlobalProcessing::shouldPromoteLocalToGlobal on the module being compiled will complain when it doesn't find summaries for the GVs in that module. There might be other places too, that's the first one I found when I was thinking about this approach.

This revision was automatically updated to reflect the committed changes.