Page MenuHomePhabricator

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

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

Diff Detail

Repository
rL LLVM

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 ↗(On Diff #214288)

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

21 ↗(On Diff #214288)

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

41 ↗(On Diff #214288)

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! :)