<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 11:41 AM David Blaikie via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</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"><div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 11:27 AM Fāng-ruì Sòng <<a href="mailto:maskray@google.com" target="_blank">maskray@google.com</a>> wrote:<br></div><div class="gmail_quote"><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"><div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 11:17 AM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><div class="gmail_quote"><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"><div dir="ltr"><div dir="ltr">On Fri, Jul 2, 2021 at 10:15 AM Fāng-ruì Sòng via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><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"><div dir="ltr">llvm/tools/ include some binary utilities used as replacement for GNU binutils, e.g. llvm-objcopy, llvm-symbolizer, llvm-nm.<br>In some old threads people discussed some drawbacks of using cl::opt for user-facing utilities (I cannot find them now).<br></div></blockquote><div><br>Would be good to describe some of the known drawbacks/expected benefits.<br></div></div></div></blockquote><div><br></div><div>The summary is the list of benefits:)</div></div></div></blockquote><div><br>Ah, it looks more like a list of changes, not necessarily benefits - removing certain syntaxes seems generally like a cost to me (potential to break existing users), rather than an outright benefit.<br></div></div></div></blockquote><div><br></div><div>Indeed: it isn't clear to me that these are outright "benefits".</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"><div dir="ltr"><div class="gmail_quote"><div><br>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.<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"><div dir="ltr"><div class="gmail_quote"><div>The drawback is some initial boilerplate (e.g. llvm-tblgen -gen-opt-parser-defs in CMakeLists.txt, class NmOptTable in code).</div><div>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 <a href="https://reviews.llvm.org/D105330" target="_blank">https://reviews.llvm.org/D105330</a>)</div><div>But this doesn't tend to increase complexity because the cl::list<std::string> will need per-value verification anyway.</div><div> <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"><div dir="ltr"><div class="gmail_quote"><div>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.</div></div></div></blockquote><div><br></div><div>OptTable is used as a local variable. So yes, it avoids global constructors, </div></div></div></blockquote><div><br></div><div>Nice :)</div></div></div></blockquote><div><br></div><div>Note that MLIR is using cl::opt without global ctor (we build with `-Werror=global-constructors`).</div><div><br></div><div>The pattern we use to write a tool with cl::opt and avoid global ctor (and can be used to avoid collision) looks like: <a href="https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83">https://github.com/llvm/llvm-project/blob/main/mlir/lib/IR/MLIRContext.cpp#L57-L83</a></div><div><br></div><div>The tool that wants to expose the MLIRContext options to the command line calls registerMLIRContextCLOptions() before parsing the command line.</div><div>Wouldn't this translate directly to LLVM tools as well with some minor refactoring?</div><div><br></div><div>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.</div><div><br></div><div>-- </div><div>Mehdi</div><div><br></div><div><br></div><div><br></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"><div dir="ltr"><div class="gmail_quote"><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"><div dir="ltr"><div class="gmail_quote"><div>thus avoiding cl::opt option name collision.</div><div>"If we decide to support binary utility multiplexing" below mentioned this point.</div><div><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"><div dir="ltr"><div class="gmail_quote"><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"><div dir="ltr">Switching to OptTable is an appealing solution. I have prepared two patches for two binary utilities: llvm-nm and llvm-strings.<br><br>* llvm-strings <a href="https://reviews.llvm.org/D104889" target="_blank">https://reviews.llvm.org/D104889</a><br>* llvm-nm <a href="https://reviews.llvm.org/D105330" target="_blank">https://reviews.llvm.org/D105330</a><br><br>llvm-symbolizer was switched last year. llvm-objdump was switched by thakis earlier this year.<br><br>The switch can fix some corners with lib/Support/CommandLine.cpp. Here is a summary:<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 option or use --no-demangle instead.<br>* 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)<br>* grouped short options can be specified with one line `setGroupedShortOptions`, instead of adding cl::Grouping to every short options.<br>* 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 <a href="https://reviews.llvm.org/D104363" target="_blank">https://reviews.llvm.org/D104363</a>)<br>* If we decide to support binary utility multiplexting (<a href="https://reviews.llvm.org/D104686" target="_blank">https://reviews.llvm.org/D104686</a>), we will not get conflicting options. An option may have different meanings in different utilities (especially for one-letter options).<br><br><b>I expect that most users will not observe any difference.</b><br><br>There is a related topic whether we should disallow the single-dash `-long-option` form.</div></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"><div dir="ltr">(Discussed in 2019: <a href="https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html" target="_blank">https://lists.llvm.org/pipermail/llvm-dev/2019-April/131786.html</a> Accept --long-option but not -long-option for llvm binary utilities)<br><b>I'd like to disallow -long-option but may want to do this in a separate change.</b></div></blockquote><div><br>I'd say definitely do this as a separate change. I expect there'd be a long tail of users after this change ships in an LLVM release, etc, such that we may want to undo some amount of it a long time after the change is made.<br></div></div></div></blockquote><div><br></div><div>Thanks for chiming in. </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"><div dir="ltr"><div class="gmail_quote"><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"><div dir="ltr"><br>The main point is that (1) grouped short options have syntax conflict with one-dash long options. (2) the GNU getopt_long style two-dash long option is much more popular.<br><br>I can think of potential pushback for some Mach-O specific options, e.g. nm -arch<br><a href="http://www.manpagez.com/man/1/nm/osx-10.12.6.php" target="_blank">http://www.manpagez.com/man/1/nm/osx-10.12.6.php</a> says `-arch` has one dash.<br>If such options may have problems, we can keep supporting one dash forms.<br>With OptTable, allowing one-dash forms for a specific option is easy.<br></div><br></blockquote></div></div></blockquote></div></div>
</blockquote></div></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div></div>