This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO][test] Add tests for emitting files in-process
ClosedPublic

Authored by Northbadge on Jun 28 2022, 4:44 PM.

Details

Summary

Completes D127777 by adding llvm-side tests for emitting index and imports files from in-process ThinLTO

Diff Detail

Event Timeline

Northbadge created this revision.Jun 28 2022, 4:44 PM
Northbadge requested review of this revision.Jun 28 2022, 4:44 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2022, 4:44 PM

lgtm, but please wait for tejohnson's feedback

Thanks! Couple minor comments.

llvm/test/ThinLTO/X86/emit-inprocess-files.ll
8

Also, this comment isn't quite right - the minimized bitcode is not just minus the debug metadata. It is without any of the IR other than the minimal symbol info needed for the summary. Better to just say "as well as minimized bitcode containing just the summary"

11

Feel free to remove this if we are already testing the minimized bitcode option elsewhere.

llvm/tools/llvm-lto2/llvm-lto2.cpp
85

As suggested below, we should check this for the distributed indexes case too. Also, either change the wording to something like "Has no effect unless specified with ...", or do some error checking.

313–314

For completeness, this should also use the new ThinLTOEmitImports. But then you will unfortunately need to update a number of existing tests (hopefully not too many as we don't tend to look at the contents or for the existence of the imports files though - a quick search suggests maybe only llvm/test/ThinLTO/X86/emit_imports.ll).

Northbadge marked 4 inline comments as done.

Thanks for the review! Added support for -thinlto-emit-imports with -thinlto-distributed-indexes

This revision is now accepted and ready to land.Jun 29 2022, 1:47 PM
This revision was landed with ongoing or failed builds.Jun 29 2022, 2:43 PM
This revision was automatically updated to reflect the committed changes.