This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] Support full list of bfd targets that lld uses.
ClosedPublic

Authored by rupprecht on Apr 16 2019, 6:38 AM.

Details

Summary

This change takes the full list of bfd targets that lld supports (see ScriptParser.cpp), including generic handling for *-freebsd targets (which uses the same settings but with a FreeBSD OSABI). In particular this adds mips support for --output-target (but not yet via --binary-architecture).

lld and llvm-objcopy use their own different custom data structures, so I'd prefer to check this in as-is (add support directly in llvm-objcopy, including all the test coverage) and do a separate NFC patch(s) that consolidate the two by putting this mapping into libobject.

See PR41462.

Diff Detail

Repository
rL LLVM

Event Timeline

rupprecht created this revision.Apr 16 2019, 6:38 AM
rupprecht edited the summary of this revision. (Show Details)Apr 16 2019, 6:39 AM
arichardson accepted this revision.Apr 16 2019, 7:21 AM

Thanks for fixing this! LGTM

llvm/tools/llvm-objcopy/CopyConfig.cpp
277 ↗(On Diff #195367)

All of these now use ELF::ELFOSABI_NONE. I wonder if we can remove the ELF::ELFOSABI_NONE from the initializer by adding a constructor with three arguments or making it the last member of the struct and adding a default initializer.

This revision is now accepted and ready to land.Apr 16 2019, 7:21 AM
jhenderson added inline comments.Apr 16 2019, 7:36 AM
llvm/tools/llvm-objcopy/CopyConfig.cpp
277 ↗(On Diff #195367)

+1 to this. That was how it used to be before I added the freebsd support recently.

MaskRay added inline comments.Apr 16 2019, 7:56 AM
llvm/tools/llvm-objcopy/CopyConfig.cpp
274 ↗(On Diff #195367)

Typo. consolidate

rupprecht updated this revision to Diff 195390.Apr 16 2019, 8:38 AM
rupprecht marked 4 inline comments as done.
  • Fix typos
  • Use alternative constructor to avoid specifying OSABI_NONE all the time
rupprecht added inline comments.Apr 16 2019, 8:43 AM
llvm/tools/llvm-objcopy/CopyConfig.cpp
277 ↗(On Diff #195367)

Sure... I ended up with three constructors, in part because we need a default constructor to have a plain "MachineInfo BinaryFormat" member of CopyConfig. The constructor is a little ugly but it does make this part look nicer :)

jhenderson added inline comments.Apr 16 2019, 8:58 AM
llvm/tools/llvm-objcopy/CopyConfig.h
33–34 ↗(On Diff #195390)

Where is this constructor still used?

This revision was automatically updated to reflect the committed changes.
rupprecht added inline comments.Apr 17 2019, 12:46 AM
llvm/tools/llvm-objcopy/CopyConfig.h
33–34 ↗(On Diff #195390)

Missed this when committing: this is shared by the two constructors below. It's a bit speculative; it assumes one might want to explicitly create a MachineInfo with a non-default OSABI. Perhaps we should remove it when extracting this+the same logic in lld into libObject.

jhenderson added inline comments.Apr 17 2019, 2:08 AM
llvm/tools/llvm-objcopy/CopyConfig.h
33–34 ↗(On Diff #195390)

Yes, it would be easy enough to explicitly set the defaults inline, without the need for a shared constructor.