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

Fri Jul 2 14:03:10 PDT 2021

On Fri, Jul 2, 2021 at 1:27 PM Fāng-ruì Sòng <maskray at google.com> wrote:

> 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.
>

You're not answering my question here, are you? Are you answering to what I
mentioned 3 emails before in an answer to David when I wrote "Indeed: it
isn't clear to me that these are outright "benefits"."?

Because I still don't see clearly how to build something like `opt` with
all the pass and the options with OptTable, how does it all compose?

Thanks,

-- 
Mehdi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210702/14f17d76/attachment.html>