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

> On Aug 18, 2014, at 3:09 PM, Rafael Espíndola <rafael.espindola at
gmail.com> wrote:
> 
>> Some passes take options directly in the constructor. For example
>> 
>> Inliner::Inliner(char &ID, int Threshold, bool InsertLifetime)
>> 
>> Maybe we could just say that there are two different types of options.
>> The ones we want to expose to users and the ones which we use for
>> testing llvm itself. The options we want to expose should be just
>> constructor arguments. With that distinction we should be able to just
>> not use the options added by  cl::OptionRegistry::CreateOption unless
>> cl::ParseCommandLineOptions is called. WebKit like clients would just
>> not call cl::ParseCommandLineOptions. Would that work?
>> 
>> 
>> This is actually how some of our internal clients are already working.
There
>> are a few caveats with this approach:
>> 
>> (1) You can’t allow the pass manager to allocate your passes for you,
>> because those passes only read from cl::opts
> 
> You mean PassManagerBuilder, right?
Yes.
> 
>> (2) Not all of our passes have constructors for overriding their
cl::opts
>> (the legacy Scalarizer is one)
>> 
>> I think it would in general be cleaner to provide a way for library
clients
>> to use cl::opts without being forced to parse a command line.
> 
> I guess it really depends on how many options there are that we want
> to expose via an API. I have the impression that there are few, which
> would make changing the constructors and PassManagerBuilder better.
> 
> If there is a large number of options that we want to expose, then I
> can see the value of having a llvm "configuration object" that is
> passed around and is queried by the passes. If we do go down this
> road, we should change passes like the inliner to use the
> configuration object instead of constructor options. We should also
> drop the "cl" from the names if it is not going to be handling
command
> lines :-)
I’m curious if Tom Stellard or Filip Pizlo have any input on this as they are
more directly involved on the client side.

I do agree that we should ultimately drop the cl namespace if we’re going in
this direction.

-Chris
> 
> Cheers,
> Rafael

> On Aug 18, 2014, at 3:21 PM, Chris Bieneman <beanz at apple.com>
wrote:
> 
> 
>>> On Aug 18, 2014, at 3:09 PM, Rafael Espíndola <rafael.espindola
at gmail.com> wrote:
>>> 
>>> Some passes take options directly in the constructor. For example
>>> 
>>> Inliner::Inliner(char &ID, int Threshold, bool InsertLifetime)
>>> 
>>> Maybe we could just say that there are two different types of
options.
>>> The ones we want to expose to users and the ones which we use for
>>> testing llvm itself. The options we want to expose should be just
>>> constructor arguments. With that distinction we should be able to
just
>>> not use the options added by  cl::OptionRegistry::CreateOption
unless
>>> cl::ParseCommandLineOptions is called. WebKit like clients would
just
>>> not call cl::ParseCommandLineOptions. Would that work?
>>> 
>>> 
>>> This is actually how some of our internal clients are already
working. There
>>> are a few caveats with this approach:
>>> 
>>> (1) You can’t allow the pass manager to allocate your passes for
you,
>>> because those passes only read from cl::opts
>> 
>> You mean PassManagerBuilder, right?
> 
> Yes.
> 
>> 
>>> (2) Not all of our passes have constructors for overriding their
cl::opts
>>> (the legacy Scalarizer is one)
>>> 
>>> I think it would in general be cleaner to provide a way for library
clients
>>> to use cl::opts without being forced to parse a command line.
>> 
>> I guess it really depends on how many options there are that we want
>> to expose via an API. I have the impression that there are few, which
>> would make changing the constructors and PassManagerBuilder better.
>> 
>> If there is a large number of options that we want to expose, then I
>> can see the value of having a llvm "configuration object"
that is
>> passed around and is queried by the passes. If we do go down this
>> road, we should change passes like the inliner to use the
>> configuration object instead of constructor options. We should also
>> drop the "cl" from the names if it is not going to be
handling command
>> lines :-)
> 
> I’m curious if Tom Stellard or Filip Pizlo have any input on this as they
are more directly involved on the client side.
The fewer options we fiddle with, the better for WebKit. Hence we would be fine
with a solution that exposes relatively few options.

The main option that we use now - turning on stack map liveness calculation - is
something that feels like it shouldn't be an "option" at all but
maybe an attribute instead.
> 
> I do agree that we should ultimately drop the cl namespace if we’re going
in this direction.
> 
> -Chris
> 
>> 
>> Cheers,
>> Rafael
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

> On Aug 18, 2014, at 4:25 PM, Filip Pizlo <fpizlo at apple.com> wrote:
> 
> 
> 
>> On Aug 18, 2014, at 3:21 PM, Chris Bieneman <beanz at apple.com>
wrote:
>> 
>> 
>>>> On Aug 18, 2014, at 3:09 PM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
>>>> 
>>>> Some passes take options directly in the constructor. For
example
>>>> 
>>>> Inliner::Inliner(char &ID, int Threshold, bool
InsertLifetime)
>>>> 
>>>> Maybe we could just say that there are two different types of
options.
>>>> The ones we want to expose to users and the ones which we use
for
>>>> testing llvm itself. The options we want to expose should be
just
>>>> constructor arguments. With that distinction we should be able
to just
>>>> not use the options added by  cl::OptionRegistry::CreateOption
unless
>>>> cl::ParseCommandLineOptions is called. WebKit like clients
would just
>>>> not call cl::ParseCommandLineOptions. Would that work?
>>>> 
>>>> 
>>>> This is actually how some of our internal clients are already
working. There
>>>> are a few caveats with this approach:
>>>> 
>>>> (1) You can’t allow the pass manager to allocate your passes
for you,
>>>> because those passes only read from cl::opts
>>> 
>>> You mean PassManagerBuilder, right?
>> 
>> Yes.
>> 
>>> 
>>>> (2) Not all of our passes have constructors for overriding
their cl::opts
>>>> (the legacy Scalarizer is one)
>>>> 
>>>> I think it would in general be cleaner to provide a way for
library clients
>>>> to use cl::opts without being forced to parse a command line.
>>> 
>>> I guess it really depends on how many options there are that we
want
>>> to expose via an API. I have the impression that there are few,
which
>>> would make changing the constructors and PassManagerBuilder better.
>>> 
>>> If there is a large number of options that we want to expose, then
I
>>> can see the value of having a llvm "configuration object"
that is
>>> passed around and is queried by the passes. If we do go down this
>>> road, we should change passes like the inliner to use the
>>> configuration object instead of constructor options. We should also
>>> drop the "cl" from the names if it is not going to be
handling command
>>> lines :-)
>> 
>> I’m curious if Tom Stellard or Filip Pizlo have any input on this as
they are more directly involved on the client side.
> 
> The fewer options we fiddle with, the better for WebKit. Hence we would be
fine with a solution that exposes relatively few options.
> 
> The main option that we use now - turning on stack map liveness calculation
- is something that feels like it shouldn't be an "option" at all
but maybe an attribute instead.
How do you construct you PassManager?

-Chris
> 
>> 
>> I do agree that we should ultimately drop the cl namespace if we’re
going in this direction.
>> 
>> -Chris
>> 
>>> 
>>> Cheers,
>>> Rafael
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>        
http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
<http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140818/cec8c3c6/attachment.html>