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

[Reid Kleckner via llvm-dev]

You know, you're right. I'm sorry, I should slow down and read more
carefully. Carry on.

On Wed, Mar 18, 2020 at 3:46 PM David Blaikie <dblaikie at gmail.com> wrote:

>
>
> 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/ad9ee409/attachment.html>