<div dir="ltr">You know, you're right. I'm sorry, I should slow down and read more carefully. Carry on.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 18, 2020 at 3:46 PM David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 18, 2020 at 3:17 PM Reid Kleckner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This change seems at odds with past design decisions. I thought we had established a goal of reducing our reliance on static initialization.</div></blockquote><div><br>Seems like it /sort/ of does that - at least in that it removes namespace-scoped static storage duration declarations from LLVM library code & means only users who create a global instance of the registration object get global ctors running?<br><br>Yeah, it doesn't exactly move us towards a point where no global ctors are used, but if it's only in the executables/put next to a "main" it doesn't seem to be a bad thing, I think? (doesn't adversely effect startup time of JITs/apps that use LLVM as a library and have no need for command line flag support) But neither did the previous solution.<br><br>Not sure if this makes it less likely someone will abuse the mechanic/make the same mistake as had happened before & declare the registration object at namespace scope in a library rather than only in executables.<br> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div><div>I believe the intention of defining these flags in a header was to make it possible to link against LLVM without automatically registering them, but to allow opt and llc to share the same command line flags without code duplication. At least, that was the problem Nadav was solving when this header was originally added:</div><div><a href="https://github.com/llvm/llvm-project/commit/e10328737db3f0e6a1a668495e4971185705d61d" target="_blank">https://github.com/llvm/llvm-project/commit/e10328737db3f0e6a1a668495e4971185705d61d</a>   <br></div><div><br></div><div>I think in practice, as indicated by the reports in the bug about lld and libclang-cpp, users actually want to make a program that behaves "just like clang", so that -mllvm flags can be parsed. There are probably only a few clients sensitive to the dynamic initialization that you have just imposed on them, but I want to bring it up, because I vaguely recall that we cared about it in the past.<br><div><br></div><div></div><div>Looking back, I think these options should've been provided from a real library used by opt and llc, and not just defined in a header. If this were landing today, I would suggest that these options get added as some library inside lib/Frontend instead of lib/CodeGen.</div></div><div><br></div><div>I don't personally feel strongly about adding this dynamic initialization, so I won't insist that you redesign things. I just want to try to provide some context from past discussions. You can try searching llvm-dev for "static initialization" to see some more of the history.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Mar 18, 2020 at 4:12 AM Serge Guelton via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Folks,<br>
<br>
Commit ac1d23ed7de01fb3a18b340536842a419b504d86 introduces a change in the way<br>
CodeGen and MC CommandFlags are handled. It's a change that may impact some<br>
devs, so I'd better give a small notice here.<br>
<br>
Basically previous approach was to bundle all options in a .inc file that<br>
declares a bunch of llvm::cl options. This file was lying in include/llvm and<br>
was to be included in client code. To avoid duplicate registration of llvm::cl<br>
options, this was meant to be only included in binaries, and not libraries.<br>
<br>
This rule of thumb was no respected by at least libclang-cpp and liblldCommon,<br>
which led to situation like this:<br>
<a href="https://bugzilla.redhat.com/show_bug.cgi?id=1756977#c5" rel="noreferrer" target="_blank">https://bugzilla.redhat.com/show_bug.cgi?id=1756977#c5</a><br>
<br>
And overall, having static options registred in « header » file is not the<br>
panacea.<br>
<br>
Current situation has moved to a more robust approach. options are registered<br>
through static objects declared in the constructor of<br>
codegen::RegisterCodeGenFlags that lies in libLLVMCodeGen. That way, when a static instance of<br>
codegen::RegisterCodeGenFlags is created, all options are registered, and they<br>
can only be registered once. If codegen::RegisterCodeGenFlags is not created,<br>
the options are not registered.<br>
<br>
Note that this approach could be used for *all* cl options.<br>
<br>
<br>
_______________________________________________<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>
_______________________________________________<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>
</blockquote></div>