[llvm-dev] Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime registration

[David Blaikie via llvm-dev]

On Wed, Mar 18, 2020 at 3:17 PM Reid Kleckner via llvm-dev <
llvm-dev at lists.llvm.org> wrote:

> This change seems at odds with past design decisions. I thought we had
> established a goal of reducing our reliance on static initialization.
>

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?

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.

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.


>
> 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:
>
> https://github.com/llvm/llvm-project/commit/e10328737db3f0e6a1a668495e4971185705d61d
>
>
> 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.
>
> 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.
>
> 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.
>
> On Wed, Mar 18, 2020 at 4:12 AM Serge Guelton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi Folks,
>>
>> Commit ac1d23ed7de01fb3a18b340536842a419b504d86 introduces a change in
>> the way
>> CodeGen and MC CommandFlags are handled. It's a change that may impact
>> some
>> devs, so I'd better give a small notice here.
>>
>> Basically previous approach was to bundle all options in a .inc file that
>> declares a bunch of llvm::cl options. This file was lying in include/llvm
>> and
>> was to be included in client code. To avoid duplicate registration of
>> llvm::cl
>> options, this was meant to be only included in binaries, and not
>> libraries.
>>
>> This rule of thumb was no respected by at least libclang-cpp and
>> liblldCommon,
>> which led to situation like this:
>> https://bugzilla.redhat.com/show_bug.cgi?id=1756977#c5
>>
>> And overall, having static options registred in « header » file is not the
>> panacea.
>>
>> Current situation has moved to a more robust approach. options are
>> registered
>> through static objects declared in the constructor of
>> codegen::RegisterCodeGenFlags that lies in libLLVMCodeGen. That way, when
>> a static instance of
>> codegen::RegisterCodeGenFlags is created, all options are registered, and
>> they
>> can only be registered once. If codegen::RegisterCodeGenFlags is not
>> created,
>> the options are not registered.
>>
>> Note that this approach could be used for *all* cl options.
>>
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200318/dba00e41/attachment.html>