This is an archive of the discontinued LLVM Phabricator instance.

[LLD][ELF] Use offset in thin archives to disambiguate thinLTO members
ClosedPublic

Authored by hoyFB on May 13 2020, 9:33 AM.

Details

Summary

This is fixing a thinLTO module collision issue for thin archives. The problem is that we always use a zero offset to name members in a thin archive and that causes the following build error:

ld.lld: error: Expected at most one ThinLTO module per bitcode file

which happens to a thin archive that has two members with the same object file name (whose paths will be ignored by thinLTO driver)

The fix here is to use real member offset instead as is done for non-thin archives.

Diff Detail

Unit TestsFailed

Event Timeline

hoyFB created this revision.May 13 2020, 9:33 AM
hoyFB edited the summary of this revision. (Show Details)May 13 2020, 9:34 AM
hoyFB added reviewers: ruiu, wenlei.
hoyFB retitled this revision from [LLD][ELF] Use offset in thin-archive to disambiguate thinLTO members to [LLD][ELF] Use offset in thin archives to disambiguate thinLTO members.May 13 2020, 9:45 AM
MaskRay added inline comments.May 13 2020, 10:42 AM
lld/test/ELF/lto/thinlto-thinarchivecollision.ll
2 ↗(On Diff #263755)

%T is currently deprecated.

You can use RUN: rm -fr %t && mkdir %t && cd %t and update the following lines to be relative to %t

23 ↗(On Diff #263755)

Rename to _start to avoid --entry=main

28 ↗(On Diff #263755)

No newline at end of file

hoyFB updated this revision to Diff 263794.May 13 2020, 11:08 AM
hoyFB marked 3 inline comments as done.
hoyFB edited the summary of this revision. (Show Details)

Updating D79880: [LLD][ELF] Use offset in thin archives to disambiguate thinLTO members

lld/test/ELF/lto/thinlto-thinarchivecollision.ll
2 ↗(On Diff #263755)

Good idea, it also simplifies the CHECK lines.

hoyFB marked an inline comment as done.May 13 2020, 11:09 AM
MaskRay accepted this revision.May 13 2020, 11:38 AM

Looks good to me. Will be worth waiting a day or so to see if there are any further comments.

lld/test/ELF/lto/thinlto-thinarchivecollision.ll
1 ↗(On Diff #263794)

thinlto-thin-archive-collision.ll

8 ↗(On Diff #263794)

--save-temps

This revision is now accepted and ready to land.May 13 2020, 11:38 AM
hoyFB updated this revision to Diff 263813.May 13 2020, 12:01 PM
hoyFB marked 2 inline comments as done.

Updating D79880: [LLD][ELF] Use offset in thin archives to disambiguate thinLTO members

hoyFB updated this revision to Diff 263881.May 13 2020, 4:21 PM

Updating D79880: [LLD][ELF] Use offset in thin archives to disambiguate thinLTO members

hoyFB marked an inline comment as done.May 13 2020, 4:23 PM
hoyFB added inline comments.
lld/test/ELF/lto/thinlto.ll
65

Attempting to make it work on Windows platform which looks like the diff time testing is performed on and with the failure:

$ ":" "RUN: at line 65"
$ "ls" "C:\ws\prod\llvm-project\build\tools\lld\test\ELF\lto\Output\thinlto.ll.tmp.dir/t.a\(t.o\" "at\" "*\).0.preopt.bc"
# command stderr:
s: C:\ws\prod\llvm-project\build\tools\lld\test\ELF\lto\Output\thinlto.ll.tmp.dir/t.a\(t.o\: No such file or directory
ls: at\: No such file or directory
l.s: *\).0.preopt.bc: Invalid argument
ruiu accepted this revision.May 13 2020, 8:29 PM

LGTM

lld/test/ELF/lto/thinlto.ll
65

I think that on Windows only double-quotes are recognized as a quotation mark and single-quotes are just regular characters. So you can probably change ' to ". Using * is fine though.

This revision was automatically updated to reflect the committed changes.