This is an archive of the discontinued LLVM Phabricator instance.

Add support for LTO plugin option obj-path
ClosedPublic

Authored by rdhindsa on May 8 2018, 1:35 PM.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

rdhindsa created this revision.May 8 2018, 1:35 PM
pcc added inline comments.May 8 2018, 1:41 PM
lld/ELF/LTO.cpp
282

You don't need this part any more. With thinlto-index-only, the obj-path file is not a "temporary" file but rather one of the final outputs.

302–307

I'd drop this part. We only need to support obj-path with thinlto-index-only.

rdhindsa updated this revision to Diff 145777.May 8 2018, 1:59 PM
rdhindsa marked 2 inline comments as done.
pcc added inline comments.May 8 2018, 2:04 PM
lld/ELF/LTO.cpp
283

I wouldn't save the buffer at all if obj-path is not passed.

rdhindsa added inline comments.May 8 2018, 2:09 PM
lld/ELF/LTO.cpp
283

Wouldn't we want to store the generated file, if obj-path is not passed? Since we know that Buff[0] is not empty, aren't we supposed to store the content? In that case, is obj-path a requirement for thinlto-index-only?

pcc added inline comments.May 8 2018, 2:19 PM
lld/ELF/LTO.cpp
283

If you happen to know that all input files do not use full LTO then there is no need to pass obj-path. The linker shouldn't write a file to an arbitrary location in that case.

rdhindsa updated this revision to Diff 145791.May 8 2018, 2:37 PM

Removed the test which ensured that regular LTO file was written with index only option. It needs to have obj-path option to write that file.

rdhindsa marked an inline comment as done.May 8 2018, 2:38 PM
pcc accepted this revision.May 8 2018, 3:36 PM

LGTM

This revision is now accepted and ready to land.May 8 2018, 3:36 PM
This revision was automatically updated to reflect the committed changes.
ruiu added a comment.May 8 2018, 3:44 PM

Please consider splitting "thinlto.ll" test file as it contains all tests for thinlto, and that's not desirabl, as small test files are more manageable than one large test file.