[lld] r270963 - --threads is a flag, not a number

[Sean Silva via llvm-commits]

On Fri, May 27, 2016 at 8:05 AM, Rui Ueyama <ruiu at google.com> wrote:

> We could add a "-lld-" prefix or something like that to new options that
> we add to LLD, but it reminds me of web browser developers who used to be
> habitually adding "-webkit-", "-moz-" or "-ms-" prefixes to new CSS
> properties. They've now reached the conclusion that that was not a good
> idea; until properties are standardized web developers had to list the same
> properties with all these different prefixes, and once they are
> standardized, they've got to add one more to the list without prefix. That
> was crazy.
>
> CSS property names are different from command line options, but the
> situation is similar, so as long as a new option has a chance to make sense
> with other linkers, I think we should just give a generic name to the
> option.
>

I agree. For something like --reproduce we don't want it to be
--lld-reproduce (I was curious why you did that for /lldicf in COFF). But
we do want some notion that options are marked hidden / experimental if we
aren't really sure that we want to keep them long term. I think for
--reproduce we are already pretty sure we want to keep it (but also it is
very special in this regard because it is (hopefully) not an option that a
user would pass as part of a real build). --lto-newpm-passes is a better
example I think.


>
> I'm wondering why you think users shouldn't use --lto-newpm-passes or
> -mllvm. As long as they understand the risk of using it, that should be
> fine, no?
>

Yes, that is the point. Generally we can't assume that they understand the
risk (in fact, I'm sure that if you ask multiple developers, they may judge
different options as being different risks).

Maybe we can have a very simple policy: if an option appears in --help then
it is officially supported for compatibility. Otherwise, users should use
it at their own risk. I believe that we are already mostly following this.
We may need to mark some options with HelpHidden (
https://github.com/llvm-mirror/llvm/blob/master/include/llvm/Option/OptParser.td#L57)
but that doesn't seem like a big deal.
We could also label some options with a group like this:
https://github.com/llvm-mirror/clang/blob/master/include/clang/Driver/Options.td#L146

-- Sean Silva


>
> On Thu, May 26, 2016 at 10:25 PM, Sean Silva <chisophugis at gmail.com>
> wrote:
>
>> This is for compatibilty with gold.
>> http://manpages.ubuntu.com/manpages/precise/man1/ld.1.html
>>
>> But this does point out something important: we don't have a clear
>> delineation between options that are LLD-specific and ones that are for
>> compatibility.
>>
>> Rui, what do you think about adopting some convention about lld-specific
>> options? Some of them are quite low level (like --lto-newpm-passes or
>> -mllvm) and I don't think we want users to be relying on them. This is
>> especially important as LLD is getting close to being let out in the wild
>> in FreeBSD and users might start relying on the options.
>>
>> -- Sean Silva
>>
>> On Thu, May 26, 2016 at 10:01 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>>> Perhaps renaming it to --multithreaded? or --parallel? (or some toher
>>> similar name - perhaps there's precedent/inspiration in the existing flag
>>> name for similar behavior in other (LLVM or non-LLVM) tools?)
>>>
>>> On Thu, May 26, 2016 at 9:30 PM, Sean Silva via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: silvas
>>>> Date: Thu May 26 23:30:27 2016
>>>> New Revision: 270963
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=270963&view=rev
>>>> Log:
>>>> --threads is a flag, not a number
>>>>
>>>> We would previously accept `--threads=4`, but this option just turns on
>>>> threading and does not specify a number of threads.
>>>>
>>>> I ran into this by accident because I was passing `--threads=<n>` but
>>>> the number didn't seem to affect anything.
>>>>
>>>> Modified:
>>>>     lld/trunk/ELF/Options.td
>>>>
>>>> Modified: lld/trunk/ELF/Options.td
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Options.td?rev=270963&r1=270962&r2=270963&view=diff
>>>>
>>>> ==============================================================================
>>>> --- lld/trunk/ELF/Options.td (original)
>>>> +++ lld/trunk/ELF/Options.td Thu May 26 23:30:27 2016
>>>> @@ -150,7 +150,7 @@ def strip_debug : Flag<["--"], "strip-de
>>>>  def sysroot : Joined<["--"], "sysroot=">,
>>>>    HelpText<"Set the system root">;
>>>>
>>>> -def threads : Joined<["--"], "threads">;
>>>> +def threads : Flag<["--"], "threads">;
>>>>
>>>>  def trace: Flag<["--"], "trace">,
>>>>    HelpText<"Print the names of the input files">;
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160527/70fc1472/attachment.html>