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

> 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>

> On Aug 18, 2014, at 4:30 PM, Chris Bieneman <beanz at apple.com>
wrote:
> 
> 
>>> 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?
LLVMCreatePassManager() and then we add target data, add a target machine
analysis pass, and then we construct our pipeline and call LLVMRunPassManager().

-Filip
> 
> -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         http://llvm.cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140819/1e84c274/attachment.html>

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. 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.

Does this sound reasonable for a first step?

-Chris
> On Aug 19, 2014, at 8:29 AM, Filip Pizlo <fpizlo at apple.com> wrote:
> 
> 
> 
> On Aug 18, 2014, at 4:30 PM, Chris Bieneman <beanz at apple.com
<mailto:beanz at apple.com>> wrote:
> 
>> 
>>> On Aug 18, 2014, at 4:25 PM, Filip Pizlo <fpizlo at apple.com
<mailto:fpizlo at apple.com>> wrote:
>>> 
>>> 
>>> 
>>>> On Aug 18, 2014, at 3:21 PM, Chris Bieneman <beanz at
apple.com <mailto:beanz at apple.com>> wrote:
>>>> 
>>>> 
>>>>>> On Aug 18, 2014, at 3:09 PM, Rafael Espíndola
<rafael.espindola at gmail.com <mailto: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?
> 
> LLVMCreatePassManager() and then we add target data, add a target machine
analysis pass, and then we construct our pipeline and call LLVMRunPassManager().
> 
> -Filip
> 
>> 
>> -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/20140819/181f84d9/attachment.html>