This is an archive of the discontinued LLVM Phabricator instance.

[ELF] Add convenience TableGen classes to enforce two dashes for options not supported by GNU ld
ClosedPublic

Authored by MaskRay on May 4 2020, 3:04 PM.

Details

Summary

Announced on https://lists.llvm.org/pipermail/llvm-dev/2020-May/141416.html

For many options, we have to support either one or two dash to be
compatible with GNU ld. For newer and lld specific options, we can enforce strict double dashes.

Affected options:

  • --thinlto-*
  • --lto-*
  • --shuffle-sections=

This patch does not change -plugin-opt=* because clang driver passes
-plugin-opt=* and I don't intend to cause churn.

In 2000, GNU ld tried something similar
https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=e4897a3288f37d5f69e8acd256a6e83e607fe8d8

Diff Detail

Event Timeline

MaskRay created this revision.May 4 2020, 3:04 PM

Since this is a lot of options that in some cases may have been used for some time (e.g. many the -thinlto-* ones have been supported for a few years now), it might be worth an announcement on llvm-dev so no one is surprised that their build scripts need updating.

lld/ELF/Options.td
542–543

What about this one, looks like Eq takes both dash forms.

MaskRay marked an inline comment as done.May 5 2020, 4:04 PM

Since this is a lot of options that in some cases may have been used for some time (e.g. many the -thinlto-* ones have been supported for a few years now), it might be worth an announcement on llvm-dev so no one is surprised that their build scripts need updating.

(I did not the history of -thinlto-* but I can't find them referenced, probably because -plugin-opt=thinlto-* is the more common use case as gold only accepts -plugin-opt=*)

Created https://lists.llvm.org/pipermail/llvm-dev/2020-May/141416.html

lld/ELF/Options.td
542–543

I left this one because I don't have a good name for it. EqEq?

psmith added a subscriber: psmith.May 6 2020, 12:28 AM

I definitely think we should do this for all new options. I agree that -o magic ought to be changed as I can see that happening in practice. The chance that someone intending -t hinlto-index-only rather than -thinlto-index-only is much lower. I'll defer to the users of thin-lto whether they think this is important.

Will be worth mentioning in the releases notes for LLD 11.0.

MaskRay updated this revision to Diff 262790.May 7 2020, 5:13 PM
MaskRay edited the summary of this revision. (Show Details)

Add EEq
Update ReleaseNotes.rst

MaskRay edited the summary of this revision. (Show Details)May 7 2020, 5:14 PM
MaskRay marked an inline comment as done.
tejohnson accepted this revision.May 7 2020, 9:43 PM

LGTM. But you should add -omagic in the summary and release notes.

This revision is now accepted and ready to land.May 7 2020, 9:43 PM

LGTM. But you should add -omagic in the summary and release notes.

--omagic is already correct.

def omagic: Flag<["--"], "omagic">, MetaVarName<"<magic>">,
  HelpText<"Set the text and data sections to be readable and writable, do not page align sections, link against static libraries">;

It was 2000's GNU ld which changed the behavior.

psmith accepted this revision.May 8 2020, 3:09 AM

LGTM too. Thanks for the update.

This revision was automatically updated to reflect the committed changes.

LGTM. But you should add -omagic in the summary and release notes.

--omagic is already correct.

def omagic: Flag<["--"], "omagic">, MetaVarName<"<magic>">,
  HelpText<"Set the text and data sections to be readable and writable, do not page align sections, link against static libraries">;

It was 2000's GNU ld which changed the behavior.

Got it, missed the fact that this was a noop change to use the new convenience class.