[LLVMdev] [RFC] Removing static initializers for command line options

[Pete Cooper]

> On Aug 20, 2014, at 8:51 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
> 
>> FWIW, I largely agree with Rafael's position here, at least in the near
>> term.
>> 
>> The nice thing about it is that even if we don't stay there forever, it is a
>> nice incremental improvement over the current state of the world, and we can
>> actually be mindful going forward of whether the restriction it imposes (an
>> inability to use "internal" knobs from within a library context that have
>> multiple different library users in a single address space) proves to be a
>> significant on-going burden.
>> 
>> I actually disagree with this for two reasons.
>> 
>> The first is that if there are going to be changes to the code anyway to
>> remove static initializers, and we can move the storage to the pass at the
>> same time, the we should make just one change and not two.
>> 
>> The second reason is that in most cases these knobs should not be globals.
>> If I had a piece of data (not a CL::opt) in global scope, only used by one
>> pass, then I'm sure people would question why it's a global at all and move
>> it inside the class. We're treating cl::opt as special here when there's no
>> reason for the opt or the storage to be global.
>> 
>> We frown upon the use of globals, otherwise LLVM would be littered with them
>> like many other C++ code bases. I don't think cl::opts should be special at
>> all in this respect.
> 
> 
> Note that the call
> 
> cl::OptionRegistry::CreateOption<bool>(&ScalarizeLoadStore,
> "ScalarizeLoadStore",
>   "scalarize-load-store", cl::Hidden, cl::init(false),
>   cl::desc("Allow the scalarizer pass to scalarize loads and store"));
> 
> ScalarizeLoadStore can actually be a member variable as long as caller
> guarantees it is still around when the command line is parsed. I have
> no objections to doing this move in the first pass if you want to.
> 
> What I would really like to avoid for now is the
> 
> cl::OptionRegistry::GetValue<bool>("ScalarizeLoadStore”);
Ah, I totally see what you mean now.  Sorry if i had been confused before.

To be honest, I had exactly the same reservations myself, but then i looked in to how we initialize and create passes and there actually isn’t a nice way to do this right now.

The trouble is that INITIALIZE_PASS doesn’t actually construct a pass.  It actually constructs a PassInfo which has a pointer to the default constructor of the pass.  Later, if the pass manager needs to (say -scalarize was on the commandline), it will get the default constructor from PassInfo and construct the pass.

Unfortunately, this completely detaches the lifetime of the pass instance where we want to store the ScalarizeLoadStore bool, from the option code to hook in to the cl::opt stuff.  I originally wanted to do something gross like pass __offset_of(Scalarizer::ScalarizeLoadStore) to the PassInfo and hide the initialization of the option in there.  But that doesn’t work either, as there’s no guarantee you’ll even create a instance of Scalarizer, even though you could pass -scalarize-load-store to the command line.

So, we do have 2 solutions: a global variable, and a call to GetValue.  As you can see, either way isn’t perfect, but if you can think of another solution i’m sure we can discuss it.

Thanks,
Pete
> 
> Cheers,
> Rafael