Mehdi AMINI via llvm-dev
2021-Jul-02 20:10 UTC
[llvm-dev] Binary utilities: switch command line parsing from llvm::cl to OptTable (byproduct: drop -long-option?)
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? Thanks, -- Mehdi -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210702/1d29290b/attachment.html>
Fāng-ruì Sòng via llvm-dev
2021-Jul-02 20:27 UTC
[llvm-dev] Binary utilities: switch command line parsing from llvm::cl to OptTable (byproduct: drop -long-option?)
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>