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

[Reid Kleckner via llvm-dev]

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

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200318/afddf466/attachment.html>