[llvm-dev] Binary utilities: switch command line parsing from llvm::cl to OptTable (byproduct: drop -long-option?)

[Fāng-ruì Sòng via llvm-dev]

On Fri, Jul 2, 2021 at 1:10 PM Mehdi AMINI <joker.eph at gmail.com> wrote:

>
>
> On Fri, Jul 2, 2021 at 12:49 PM Alexandre Ganea <
> alexandre.ganea at ubisoft.com> wrote:
>
>> The API benefits sound nice, though presumably some could be retrofitted
>> to cl::opt if that was the only goal. Side benefits in addition to removing
>> global ctors are nice to have.
>>
>>
>> The drawback is some initial boilerplate (e.g. llvm-tblgen
>> -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code).
>>
>> The handling of comma separated options -arch=x86_64,arm64 doesn't have
>> direct OptTable support. llvm::SplitString is needed (just search for
>> SplitString in https://reviews.llvm.org/D105330)
>>
>> But this doesn't tend to increase complexity because the
>> cl::list<std::string> will need per-value verification anyway.
>>
>>
>>
>> One potential one (though I don't recall it being discussed recently)
>> would be that maybe this addresses the issue of global ctors in cl::opt?
>> Does OptTable avoid/not use global constructors? That would be nice - it's
>> an ongoing issue that LLVM library users pay for command line argument
>> support they have no need for in the form of global ctor execution time.
>>
>>
>>
>> OptTable is used as a local variable. So yes, it avoids global
>> constructors,
>>
>>
>>
>> Nice :)
>>
>>
>>
>> Note that MLIR is using cl::opt without global ctor (we build with
>> `-Werror=global-constructors`).
>>
>>
>>
>> The pattern we use to write a tool with cl::opt and avoid global ctor
>> (and can be used to avoid collision) looks like:
>> https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83
>>
>>
>>
>> The tool that wants to expose the MLIRContext options to the command line
>> calls registerMLIRContextCLOptions() before parsing the command line.
>>
>> Wouldn't this translate directly to LLVM tools as well with some minor
>> refactoring?
>>
>>
>>
>> The same applies to all of the infrastructure in MLIR, passes are
>> registered explicitly, etc. This decouples the "is this code linked in"
>> from "options are loaded" annoying part of the global constructors.
>>
>>
>>
>> --
>>
>> Mehdi
>>
>>
>>
>> *[Alexandre Ganea] *I think one other issue with cl::opt is that it
>> aggregates the “command-line argument definition” and the “runtime
>> parameter” *de facto* in a single object (unless cl::location is
>> manually specified to every cl::opt). What MLIR does solves the issue
>> mentioned by David, the fact that every tool pulls/initializes every
>> cl::opt out there. However OptTable solves both problems, and makes the
>> entry point thread-safe.
>>
>>
>>
>
> I agree that removing the global state would be great!
> Right now what I see proposed with OptTable (like
> https://reviews.llvm.org/D104889) seems to just address the
> tools-specific options, and the value isn't clear to me for these cases,
> since these options aren't exposed through library entry points.
> I don't quite get right now how OptTable would compose at the LLVM scale?
> Are there examples of libraries exposing pluggable hooks for a tool to
> aggregate multiple libraries' options and expose them on the command line?
>
>
The first message listed:

* -t=d is removed (equal sign after a short option). Use -t d instead.
* --demangle=0 (=0 to disable a boolean option) is removed. Omit the option
or use --no-demangle instead.
* To support boolean options (e.g. --demangle --no-demangle), we don't need
to compare their positions (if (NoDemangle.getPosition() >
Demangle.getPosition()) , see llvm-nm.cpp)
* grouped short options can be specified with one line
`setGroupedShortOptions`, instead of adding cl::Grouping to every short
options.
* We don't need to add cl::cat to every option and call
`HideUnrelatedOptions` to hide unrelated options from --help. The issue
would happen with cl::opt tools if linker garbage collection is disabled or
libLLVM-13git.so is used. (See https://reviews.llvm.org/D104363)

To me *these points are all usability issues of cl::opt*. I care about not
exposing unnecessary interfaces so cl::opt accepting the weird -t=d looks a
downside to me.

--demangle=0 is weird and some llvm/llvm/test tests do use cl::opt options
this way, so we cannot just remove this usage. As a workaround, we could
add a cl::foobar_toggle to a cl::opt to disallow =0.
We would end with more customization for one option, cl::cat (for hiding
unrelated options), cl::foobar_toggle (for disallowing =0), and potentially
others for other ad-hoc tasks.

I can highlight another thing about the global state of cl::opt => *library
cl::opt and binary utility cl::opt share the same namespace*.
So cl::opt options (usually for debugging or testing) in library code can
end up in a tool's list of command line options.
This is usually undesired (e.g. llvm-objdump --x86-asm-syntax in
https://reviews.llvm.org/D100433).
People may not notice this if they always use -DLLVM_LINK_LLVM_DYLIB=off
and don't use linker garbage collection.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210702/bf5244c4/attachment.html>