<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 2, 2021 at 4:49 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">[This topic of this subthread has shifted from binary utilities to generic library options]<br>
<br>
> Sure, but that isn't the problem you were raising that I was answering<br>
> to, let's not move the goal post here.<br>
> <br>
> How do you solve this "namespacing" issue though? This was the sense of my<br>
> question earlier in this thread: is OptTable providing me a solution to<br>
> build library options that can be re-exported through tools command line<br>
> interface? (bonus point if it manages some sort of namespacing).<br>
<br>
No, OptTable options are not composable.<br>
<br>
> The intent of cl::opt in libraries is that they can be exposed on the<br>
> command line. It seems to me that this namespacing issue is quite intrinsic<br>
> to the flat command line interface itself.<br>
<br>
Yes.<br>
<br>
> The way we work around this to avoid collision in mlir is through<br>
> convention, making sure passes are prefixed according to their library (or<br>
> dialects in MLIR).<br>
<br>
Yes. llvm-readobj has a similar pattern: calling a function which<br>
defines cl::opt local variables.<br>
<br>
<br>
OptTable is for tools like clang, flang, lld where<br>
<br>
* they want high configurability, e.g. -long-option for some options while --long-option for others.<br>
* they don't want library cl::opt options.[1]<br>
* they want to avoid cl::opt's loosing behavior (-t=d and -demangle=0 are two examples I raised; hey, -enable-new-pm=0:) it is convenient internally but externally we'd better use -no- instead)<br>
<br>
[1]: As you may have implied, this<br>
is alternatively solvable by reorganizing llvm/lib/* options. This is<br>
a huge task and the solution isn't particular clear yet.<br>
--riscv-no-aliases is an example that cl::opt in library code can affect<br>
multiple tools: llc/llvm-mc/llvm-objdump (llvm-mc/llvm-objdump usage was<br>
unintentional). Someone signing up for the work needs to be careful on<br>
letting utilities call the relevant register functions.<br>
<br>
My proposal is like: binary utilities should move in the direction of<br>
clang/lld as well.<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><br>
> Another thing we have in MLIR is cl::opt wrapped into external storage and<br>
> custom parser, they aren't exposed in the global namespace.<br>
> For example, a pass class will define a member:<br>
> <br>
>   ::mlir::Pass::Option<uint64_t> fastMemoryCapacity{*this,<br>
> "fast-mem-capacity", ::llvm::cl::desc("Set fast memory space capacity in<br>
> KiB (default: unlimited)"),<br>
> ::llvm::cl::init(std::numeric_limits<uint64_t>::max())};<br>
> <br>
> Where mlir::Pass::Option is a inheriting from cl::opt here:<br>
> <a href="https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Pass/PassOptions.h#L111" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/Pass/PassOptions.h#L111</a><br>
> <br>
> In `mlir-opt --help` it shows up as:<br>
> <br>
>   Compiler passes to run<br>
>     --pass-pipeline                                     -   A textual<br>
> description of a pass pipeline to run<br>
>     Passes:<br>
>       --affine-data-copy-generate                       -   Generate<br>
> explicit copying for affine memory operations<br>
>         --fast-mem-capacity=<ulong>                     - Set fast memory<br>
> space capacity in KiB (default: unlimited)<br>
> <br>
> Note how the  "fast-mem-capacity" is nested under the<br>
> "affine-data-copy-generate" (which is the pass name).<br>
> On the command line it won't be present at the top-level and you end up<br>
> invoking it this way:<br>
> <br>
> // RUN: mlir-opt %s -split-input-file<br>
> -affine-data-copy-generate="generate-dma=false fast-mem-space=0<br>
> skip-non-unit-stride-loops" | FileCheck %s<br>
> <br>
> It also has the advantage that you can invoke the same pass twice in the<br>
> pipeline with a different value for the same cl::opt since the storage is<br>
> private to a pass instance.<br>
> <br>
> <br>
> <br>
> <br>
> ><br>
> > This can be worked around with linker garbage collection by discarding<br>
> > unreferenced cl::opt.<br>
> ><br>
> <br>
> I don't understand how this works actually: cl::opt that rely on global<br>
> constructors aren't referenced ever, as long as the file is linked in they<br>
> will be involved. This is an incredibly clunky situation to play with<br>
> linker semantics here, we end relying on the way files are organized in<br>
> static archives (and it breaks when you build libLLVM.so as you mentioned<br>
> before).<br>
<br>
Yep, the reliance on linker garbage collection makes hiding unrelated options.<br>
<br>
On 2021-07-02, Mehdi AMINI wrote:<br>
>Looks like our email crossed :)<br>
><br>
>On Fri, Jul 2, 2021 at 4:07 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
><br>
>> On Fri, Jul 2, 2021 at 3:26 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
>><br>
>>> On Fri, Jul 2, 2021 at 3:21 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br>
>>><br>
>>>><br>
>>>><br>
>>>> On Fri, Jul 2, 2021 at 2:09 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br>
>>>><br>
>>>>> On Fri, Jul 2, 2021 at 2:03 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br>
>>>>><br>
>>>>>><br>
>>>>>><br>
>>>>>> On Fri, Jul 2, 2021 at 1:27 PM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>><br>
>>>>>> wrote:<br>
>>>>>><br>
>>>>>>> On Fri, Jul 2, 2021 at 1:10 PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>><br>
>>>>>>> wrote:<br>
>>>>>>><br>
>>>>>>>><br>
>>>>>>>><br>
>>>>>>>> On Fri, Jul 2, 2021 at 12:49 PM Alexandre Ganea <<br>
>>>>>>>> <a href="mailto:alexandre.ganea@ubisoft.com" target="_blank">alexandre.ganea@ubisoft.com</a>> wrote:<br>
>>>>>>>><br>
>>>>>>>>> The API benefits sound nice, though presumably some could be<br>
>>>>>>>>> retrofitted to cl::opt if that was the only goal. Side benefits in addition<br>
>>>>>>>>> to removing global ctors are nice to have.<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> The drawback is some initial boilerplate (e.g. llvm-tblgen<br>
>>>>>>>>> -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code).<br>
>>>>>>>>><br>
>>>>>>>>> The handling of comma separated options -arch=x86_64,arm64 doesn't<br>
>>>>>>>>> have direct OptTable support. llvm::SplitString is needed (just search for<br>
>>>>>>>>> SplitString in <a href="https://reviews.llvm.org/D105330" rel="noreferrer" target="_blank">https://reviews.llvm.org/D105330</a>)<br>
>>>>>>>>><br>
>>>>>>>>> But this doesn't tend to increase complexity because the<br>
>>>>>>>>> cl::list<std::string> will need per-value verification anyway.<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> One potential one (though I don't recall it being discussed<br>
>>>>>>>>> recently) would be that maybe this addresses the issue of global ctors in<br>
>>>>>>>>> cl::opt? Does OptTable avoid/not use global constructors? That would be<br>
>>>>>>>>> nice - it's an ongoing issue that LLVM library users pay for command line<br>
>>>>>>>>> argument support they have no need for in the form of global ctor execution<br>
>>>>>>>>> time.<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> OptTable is used as a local variable. So yes, it avoids global<br>
>>>>>>>>> constructors,<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> Nice :)<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> Note that MLIR is using cl::opt without global ctor (we build with<br>
>>>>>>>>> `-Werror=global-constructors`).<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> The pattern we use to write a tool with cl::opt and avoid global<br>
>>>>>>>>> ctor (and can be used to avoid collision) looks like:<br>
>>>>>>>>> <a href="https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83</a><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> The tool that wants to expose the MLIRContext options to the<br>
>>>>>>>>> command line calls registerMLIRContextCLOptions() before parsing the<br>
>>>>>>>>> command line.<br>
>>>>>>>>><br>
>>>>>>>>> Wouldn't this translate directly to LLVM tools as well with some<br>
>>>>>>>>> minor refactoring?<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> The same applies to all of the infrastructure in MLIR, passes are<br>
>>>>>>>>> registered explicitly, etc. This decouples the "is this code linked in"<br>
>>>>>>>>> from "options are loaded" annoying part of the global constructors.<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> --<br>
>>>>>>>>><br>
>>>>>>>>> Mehdi<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>> *[Alexandre Ganea] *I think one other issue with cl::opt is that<br>
>>>>>>>>> it aggregates the “command-line argument definition” and the “runtime<br>
>>>>>>>>> parameter” *de facto* in a single object (unless cl::location is<br>
>>>>>>>>> manually specified to every cl::opt). What MLIR does solves the issue<br>
>>>>>>>>> mentioned by David, the fact that every tool pulls/initializes every<br>
>>>>>>>>> cl::opt out there. However OptTable solves both problems, and makes the<br>
>>>>>>>>> entry point thread-safe.<br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>>><br>
>>>>>>>><br>
>>>>>>>> I agree that removing the global state would be great!<br>
>>>>>>>> Right now what I see proposed with OptTable (like<br>
>>>>>>>> <a href="https://reviews.llvm.org/D104889" rel="noreferrer" target="_blank">https://reviews.llvm.org/D104889</a>) seems to just address the<br>
>>>>>>>> tools-specific options, and the value isn't clear to me for these cases,<br>
>>>>>>>> since these options aren't exposed through library entry points.<br>
>>>>>>>> I don't quite get right now how OptTable would compose at the LLVM<br>
>>>>>>>> scale? Are there examples of libraries exposing pluggable hooks for a tool<br>
>>>>>>>> to aggregate multiple libraries' options and expose them on the command<br>
>>>>>>>> line?<br>
>>>>>>>><br>
>>>>>>>><br>
>>>>>>> The first message listed:<br>
>>>>>>><br>
>>>>>>> * -t=d is removed (equal sign after a short option). Use -t d instead.<br>
>>>>>>> * --demangle=0 (=0 to disable a boolean option) is removed. Omit the<br>
>>>>>>> option or use --no-demangle instead.<br>
>>>>>>> * To support boolean options (e.g. --demangle --no-demangle), we<br>
>>>>>>> don't need to compare their positions (if (NoDemangle.getPosition() ><br>
>>>>>>> Demangle.getPosition()) , see llvm-nm.cpp)<br>
>>>>>>> * grouped short options can be specified with one line<br>
>>>>>>> `setGroupedShortOptions`, instead of adding cl::Grouping to every short<br>
>>>>>>> options.<br>
>>>>>>> * We don't need to add cl::cat to every option and call<br>
>>>>>>> `HideUnrelatedOptions` to hide unrelated options from --help. The issue<br>
>>>>>>> would happen with cl::opt tools if linker garbage collection is disabled or<br>
>>>>>>> libLLVM-13git.so is used. (See <a href="https://reviews.llvm.org/D104363" rel="noreferrer" target="_blank">https://reviews.llvm.org/D104363</a>)<br>
>>>>>>><br>
>>>>>>> To me *these points are all usability issues of cl::opt*. I care<br>
>>>>>>> about not exposing unnecessary interfaces so cl::opt accepting the weird<br>
>>>>>>> -t=d looks a downside to me.<br>
>>>>>>><br>
>>>>>>> --demangle=0 is weird and some llvm/llvm/test tests do use cl::opt<br>
>>>>>>> options this way, so we cannot just remove this usage. As a workaround, we<br>
>>>>>>> could add a cl::foobar_toggle to a cl::opt to disallow =0.<br>
>>>>>>> We would end with more customization for one option, cl::cat (for<br>
>>>>>>> hiding unrelated options), cl::foobar_toggle (for disallowing =0), and<br>
>>>>>>> potentially others for other ad-hoc tasks.<br>
>>>>>>><br>
>>>>>>> I can highlight another thing about the global state of cl::opt => *library<br>
>>>>>>> cl::opt and binary utility cl::opt share the same namespace*.<br>
>>>>>>> So cl::opt options (usually for debugging or testing) in library code<br>
>>>>>>> can end up in a tool's list of command line options.<br>
>>>>>>> This is usually undesired (e.g. llvm-objdump --x86-asm-syntax in<br>
>>>>>>> <a href="https://reviews.llvm.org/D100433" rel="noreferrer" target="_blank">https://reviews.llvm.org/D100433</a>).<br>
>>>>>>> People may not notice this if they always<br>
>>>>>>> use -DLLVM_LINK_LLVM_DYLIB=off and don't use linker garbage collection.<br>
>>>>>>><br>
>>>>>><br>
>>>>>> You're not answering my question here, are you? Are you answering to<br>
>>>>>> what I mentioned 3 emails before in an answer to David when I wrote<br>
>>>>>> "Indeed: it isn't clear to me that these are outright "benefits"."?<br>
>>>>>><br>
>>>>><br>
>>>>> OptTable doesn't have the listed usability issues. Not having the<br>
>>>>> issues is a large benefit to me.<br>
>>>>><br>
>>>><br>
>>>> Maybe, these are subjective though.<br>
>>>> (And the confusion above came from the fact that  you just answered the<br>
>>>> wrong email)<br>
>>>><br>
>>>><br>
>>>>><br>
>>>>><br>
>>>>>> Because I still don't see clearly how to build something like `opt`<br>
>>>>>> with all the pass and the options with OptTable, how does it all compose?<br>
>>>>>><br>
>>>>><br>
>>>>> The proposed changes are specific to binary utilities:<br>
>>>>> llvm-{ar,cxxfilt,nm,objcopy,objdump,readobj,size,strings,symbolizer}.<br>
>>>>> 'opt', 'llc', etc are not in the scope.<br>
>>>>> (I guess I should have named the utilities more specifically to not<br>
>>>>> cause confusion.)<br>
>>>>><br>
>>>>> For<br>
>>>>> llvm-{ar,cxxfilt,nm,objcopy,objdump,readobj,size,strings,symbolizer}, we<br>
>>>>> don't want a random cl::opt in lib/Transforms, lib/MC, orlib/LTO to be<br>
>>>>> accidentally specifiable on the command line.<br>
>>>>> (In the rare debugging case where such functionality is needed, it<br>
>>>>> needs a -mllvm prefix like what ld.lld does.)<br>
>>>>><br>
>>>><br>
>>>> Have you looked at what I mentioned with MLIR on how to use cl::opt<br>
>>>> without global constructor? This has exactly the behavior you're looking<br>
>>>> for.<br>
>>>><br>
>>>><br>
>>> I have read<br>
>>> <a href="https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83</a><br>
>>><br>
>>> Unless it does something more specially, I don't think it can avoid the<br>
>>> issue I mentioned in a previous message:<br>
>>><br>
>>> > I can highlight another thing about the global state of cl::opt => *library<br>
>>> cl::opt and binary utility cl::opt share the same namespace*.<br>
>>><br>
>>> This can be worked around with linker garbage collection by discarding<br>
>>> unreferenced cl::opt.<br>
>>><br>
>><br>
>> I re-read these messages. I think you probably meant something more<br>
>> generic - how to design decentralized command line option registry where<br>
>> every library can register some options.<br>
>><br>
><br>
>Yes indeed!<br>
><br>
>I'm not against using OptTable for the sole purpose of binary tools, but<br>
>that does not seem to provide us with a path forward. So I am afraid that<br>
>this is a local optima that is short sighted.<br>
<br>
Mentioned previously, I don't try to alter current cl::opt usage in<br>
library code and non-binary-utility tools (e.g. opt,llc).<br>
<br>
Though I am not sure I agree with clang/lld/llvm-symbolizer/llvm-objdump<br>
are using a local optima. I think cl::opt just don't fit for their use cases.<br>
OptTable is just the appropriate solution for them.<br></blockquote><div><br></div><div><br>I think part of the confusion on my side in this thread is that when I read "binary utilities" I thought first and foremost about `opt` and `lld`, while you're using "binary utilities" to refer to what I would call "end-user tools".<br>I agree with you that tools like clang and lld are in a different category than `opt`.</div><div><br></div><div>cl::opt as it may not be suitable as-is, but OptTable being not composable and not offering any facility to someone building a tool to re-expose library options is also quite limited. It seems to me that we need such a solution, and it also seems that if we had such solutions it would be suitable for all the tools we intend to build using LLVM-based libraries.</div><div>Without any plan to build this though, I'm not trying to block progress on your cleanup/improvement of these end-user tools :)</div><div><br></div><div>Cheers,</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div> <br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
<br>
Bogus: since options are data with the OptTable approach, we can re-use<br>
something like clang-tblgen -gen-opt-docs to help ensure the documentation<br>
doesn't diverge from the reality.<br>
<br>
>> The binary utilities command line parsing issue I intended to address is<br>
>> fairly different:<br>
>> We want a single registry of all options and don't want to inherit options<br>
>> from llvm/lib/A just because the tool happens to depend on llvm/lib/A<br>
>> directly or indirectly.<br>
>><br>
><br>
>I agree, what I pointed at in MLIR was an attempt to achieve this, the tool<br>
>has to explicitly call a function at runtime to make the options available.<br>
><br>
><br>
>> E.g. llvm-nm needs to depend on bitcode reader because it needs to handle<br>
>> LLVM bitcode files, however, I don't want a random cl::opt in bitcode<br>
>> reader to appear in llvm-nm's command line option list.<br>
>><br>
>> So I just built mlir-opt and inspected its --help output. It has exactly<br>
>> the problem I called out in my first message:<br>
>><br>
><br>
>> * We don't need to add cl::cat to every option and call<br>
>><br>
>> `HideUnrelatedOptions` to hide unrelated options from --help. The issue<br>
>> would happen with cl::opt tools if linker garbage collection is disabled or<br>
>> libLLVM-13git.so is used. (See <a href="https://reviews.llvm.org/D104363" rel="noreferrer" target="_blank">https://reviews.llvm.org/D104363</a>)<br>
>><br>
>> There are many unrelated options like --amdgpu-bypass-slow-div which is from llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp<br>
>><br>
>><br>
>I think you're missing the point here: mlir-opt does not have this problem *for<br>
>mlir components.* It pays the price of the LLVM components using global<br>
>constructors, but I pointed to the solution we use for mlir components that<br>
>aren't using global constructors and avoid this problem.<br>
<br>
See [1] above <br>
</blockquote></div></div>