This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objcopy] Move duplicate tablegen from objcopy and strip into one file
ClosedPublic

Authored by mmpozulp on Aug 8 2019, 7:34 PM.

Event Timeline

mmpozulp created this revision.Aug 8 2019, 7:34 PM
mmpozulp updated this revision to Diff 214288.Aug 8 2019, 8:07 PM
mmpozulp added a subscriber: wolfgangp.

Rebase on master to get commit from @wolfgangp which added --strip-sections
to llvm-strip. Move it into CommonOpts.td since llvm-objcopy supports it to.

mmpozulp edited the summary of this revision. (Show Details)Aug 8 2019, 8:08 PM
jhenderson added inline comments.Aug 9 2019, 2:14 AM
llvm/tools/llvm-objcopy/CommonAliases.td
3 ↗(On Diff #214288)

I bet that if you adopt my changes to not use the classes below, this is unnecessary.

llvm/tools/llvm-objcopy/CommonOpts.td
15

You can avoid the need for the class here by rewording slightly to say "Allow the tool to..."

20–21

Here and in the disable case, you can avoid the class by rewording slightly to something like "when operating on archives".

47

Again, I think you can remove the reference to the tool name entirely here i.e. "GNU's --strip-all".

mmpozulp updated this revision to Diff 214422.Aug 9 2019, 12:34 PM
mmpozulp marked 4 inline comments as done.

Delete CommonAliases.td by incorporating feedback from @jhenderson.

This revision is now accepted and ready to land.Aug 12 2019, 1:58 AM
MaskRay accepted this revision.Aug 12 2019, 2:18 AM
MaskRay added a subscriber: MaskRay.

Copied from llvm/tools/llvm-objcopy/StripOpts.td

Phabricator is smart?

Copied from llvm/tools/llvm-objcopy/StripOpts.td

Phabricator is smart?

I think it's to do with how the patch is written. I'm not sure though :/

rupprecht accepted this revision.Aug 12 2019, 12:13 PM
This revision was automatically updated to reflect the committed changes.

Copied from llvm/tools/llvm-objcopy/StripOpts.td

Phabricator is smart?

I think it's to do with how the patch is written. I'm not sure though :/

Git is smart. The top of the patch says "copy from" / "copy to" https://reviews.llvm.org/file/data/ujocforfsc2wd7mdfbdg/PHID-FILE-gbjfb3ce67cfiabcoyar/D65991.id214422.diff. The manpage git-diff(1) explains the patch format but does not describe git's copying detection algorithm. Does anyone know? I'd love to find out more. @MaskRay, you have sparked my interest! :)