llvm dev - Aug 2014 - [LLVMdev] [RFC] Removing static initializers for command line options

> On Aug 19, 2014, at 11:09 AM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
> 
> On 19 August 2014 13:47, Chris Bieneman <beanz at apple.com> wrote:
>> I’d like to propose moving forward with the first phase of my proposal
to
>> make the cl::opt structures owned and eliminate global option storage.
> 
> For now, please eliminate only the static constructor and leave the
> storage. Since it seems only a few options that need to be exposed by
> non-command line APIs, we might be able to avoid the need for a
> cl::OptionRegistry::GetValue.
I strongly feel this is the wrong decision. If you have a single process using
two LLVM clients (say WebKit and Mesa), and they both are using an opt pass with
different settings. If the storage is global this will not work.
> 
>> I’d
>> also like to add to it that when updating passes I will ensure that
each
>> pass that has cl::opts also has a default constructor, an overridden
>> constructor to populate each option, and the corresponding factory
methods
>> for the C API.
> 
> And *please* don't add anything to the C api unless someone has a real
> need for that particular interface and there is a good discussion on
> the review thread about it. The C api has more backwards compatibility
> promises, which makes it incredibly painful :-(
> 
> Even the C++ api is not free, so I would also only modify a pass
> constructor if someone wants to pass that option for something other
> then llvm development or testing. For that command lines are a
> perfectly reasonable solution.
I can agree with all of this.

-Chris
> 
>> Does this sound reasonable for a first step?
> 
> Removing the static constructors does.
> 
> Cheers,
> Rafael

> I strongly feel this is the wrong decision. If you have a single process
using two LLVM clients (say WebKit and Mesa), and they both are using an opt
pass with different settings. If the storage is global this will not work.
That is only an issue if they can set the option at all.

To summarize, the point is that there are two *very* different types of options

* Nobs for which there is not a single right answer for all users.
There are very few of these currently and we expect it to remain like
that. These should not use cl::opt or static storage at all. They
should be an option passed to the constructor of the pass.  It may
also involve exposing it to the C api and the PassManagerBuilder.

* Options that we use during development of llvm. They are useful for
testing, tracking a bug or enabling/disabling a feature that is still
under development. These can use a static storage since external
clients like webkit will never change them. We do have to avoid these
options requiring static constructors, since otherwise the client is
paying for something it will never use.

Cheers,
Rafael

> On Aug 19, 2014, at 12:32 PM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
> 
>> I strongly feel this is the wrong decision. If you have a single
process using two LLVM clients (say WebKit and Mesa), and they both are using an
opt pass with different settings. If the storage is global this will not work.
> 
> That is only an issue if they can set the option at all.
> 
> To summarize, the point is that there are two *very* different types of
options
> 
> * Nobs for which there is not a single right answer for all users.
> There are very few of these currently and we expect it to remain like
> that. These should not use cl::opt or static storage at all. They
> should be an option passed to the constructor of the pass.  It may
> also involve exposing it to the C api and the PassManagerBuilder.
How would you suggest we expose cl::opts for modifying these options in tools
like opt? A good example of this type of option would be the options in
LoopUnrollPass.cpp.
> 
> * Options that we use during development of llvm. They are useful for
> testing, tracking a bug or enabling/disabling a feature that is still
> under development. These can use a static storage since external
> clients like webkit will never change them. We do have to avoid these
> options requiring static constructors, since otherwise the client is
> paying for something it will never use.
What about when we're debugging the WebKit JIT? For development of libraries
using LLVM it would be nice to be able to toggle these values too, which is why
Filip’s suggestion of an API like LLVMConfigSetBoolValue(Config,
"ScalarizeLoadStore", 1) would be nice.

-Chris
> 
> Cheers,
> Rafael