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

[Chris Bieneman]

Chandler brought up a lot of really great points. One of his big points was that if we’re going to touch every pass, we may as well fix the cl::opt API too.

Toward that end I’ve put together some patches with a new fluent-style API as a starting point of a conversation.

Disclaimer: To make these patches work with the existing cl::opts there is some nasty hackery going on in CommandLine.h — For the sake of this conversation I’d like to focus our attention on the API for creating and accessing options in Scalarizer.cpp

My intent with these changes is to provide a groundwork for meeting all of Chandler’s main points. These patches have the following high-level changes from my previous patches:

1) I’ve switched to some crazy template-foo for generating IDs to identify options
2) I’ve moved all the new API components out of the cl namespace
3) There is no option storage outside the OptionRegistry, but I have defined a get API and setup template methods for reading from a store and suggested API for LLVMContext to implement
4) I’ve defined a new API for defining options in the form of the opt_builder object. This is intended to be a transitional API only, but it allows us to migrate off the cl::opt template-foo

As with my previous patches, these patches are designed to work with existing cl::opts.

Thanks,
-Chris


> On Aug 26, 2014, at 6:18 PM, Robinson, Paul <Paul_Robinson at playstation.sony.com> wrote:
> 
> I count 15 occurrences of "-backend-option" in clang/lib/Driver/Tools.cpp and most (all? didn't look at them real hard) of those aren't debugging options.  Presumably those can all be solved by adding more plumbing, but it would be *really* nice to make that plumbing easier to do.
> --paulr
>   <>
> From: llvmdev-bounces at cs.uiuc.edu [mailto:llvmdev-bounces at cs.uiuc.edu] On Behalf Of Chandler Carruth
> Sent: Tuesday, August 26, 2014 10:42 AM
> To: Chris Bieneman
> Cc: LLVM Dev
> Subject: Re: [LLVMdev] [RFC] Removing static initializers for command line options
>  
> A bunch of IRC discussions etc have taken place, and I wanted to surface some of the points from them here. Also, I wanted to give a more fully thought-through set of my own perspectives.
>  
>  
> First, I'd like to re-state the goals as I think those have gotten muddied.
>  
> 1) Remove most of LLVM's static initializers (that is, dynamic initialization code for static storage duration objects) by moving away from global objects with non-trivial constructors of 'llvm::cl::opt' types in the LLVM *libraries* (they remain fine in the tools of course).
>  
> 2) Support separate LLVM users in a single address space having different values for the debugging knobs currently managed through the 'llvm::cl::opt' command line flags mechanism.
>  
> 3) (related to #2) Support some library API for setting the debugging knobs currently managed through the 'llvm::cl::opt' command line flags mechanism.
>  
>  
> Now, I want to change my position on one point because after talking to Chris, Jim, and others I understand the problem significantly better. I am now convinced that it is impossible to fix even #1 without making a change for every single option in LLVM. Why? Because we have to *register* all of the flags, and this registration inherently cannot be done from a global's constructor without introducing the undesirable static initializers[1].
>  
> I also think that in order to address #2 we would need to make some change to essentially every option in LLVM as we would have to connect it to non-global storage.
>  
> Given that, I think it does make sense to try to address #1 and #2 at the same time -- trying to limit the change to just one or the other would very likely result in a significantly different API for creating, registering, and using these options, and I would rather not make all of the LLVM developers re-learn how to do this 2 times if we can instead do it 1 time.
>  
>  
> However, doing #2 only really makes sense if we also do #3 to let people *use* this new freedom. =] Owen convinced me that there really is a good reason to support #3 as part of the LLVM libraries, however I wanted to raise my primary concern about this on the mailing list because it hasn't really been discussed.
>  
> These "debugging knobs" or library level options which *aren't* exposed in the library's APIs are very risky to expose in a side-channel API: they are often completely unsupported or unsupportable options that were never intended for "production" usage. Once we expose these options via some API to library users, we run a very serious risk of getting locked into supporting behaviors or debugging misbehavior that were never expected or planned for...
>  
> However, the debugging use case is real and important. So I want to propose that we guard many of the paths to setting these options with NDEBUG so that in non-asserts builds, library users can't override these options. This would mean the C++ API for setting this in LLVMContext (and the C API if we ever expose it there) would only support setting these in asserts builds. We could even sink the checking into the registry. The name should probably have "debug" in it somewhere. It would then have some InternalUnsupportedScaryBad API which is not actually in the header files but is used in LLVM's tools to maintain their current behavior without documenting a public API. It's not bulletproof, but it seemed that it would be sufficient for the debugging use cases and is a strong statement of "unsupported" IMO.
>  
>  
> The only other observation I would like to make is that if we're going to do this, we really should take the time to try to sort out a really *good* API. Hopefully we'll be slightly less wrong this time, and maybe the next transition can be more incremental.
>  
>  
> Ok, on to the more low-level parts of this email, here some specific thoughts for how to design the API for further discussion:
>  
> - I really like the idea of using a 'void*' derived from some static tag as a way to identify uniquely each option. I also think we can use some minimal "reflection" techniques to significantly simplify this.
>  
> - The need for a global registry is... distasteful, but I agree inevitable.
>  
> - I'd like to avoid a separate "option store". My current suggestion is to use the LLVMContext, and to add a context to the layers which don't have an LLVMContext and where these kinds of options make sense.
>  
> - I would actually use a simple 'void*' -> string mapping in the "store" of the context. (Maybe we can actually use StringMap?) I would still provide the type to the registry and use that to validate options when parsing. When getting the value out of the "store" in the context, we can parse it into the data-type and assert fail on parse errors. I don't feel strongly about this, but without "std::any" it seems simpler than rolling our own with a void* or a union.
>  
> - I would provide a simple API on the LLVMContext to get an "option" string for a given key. This API could be exposed by any context, and we can write the option value extraction library to accept a generic T which supports such a query method. That way there is *very* loose coupling between these and it is easy to support any context object at any layer.
>  
> - The registry should essentially provide an interface to parse an argv string list into such a map from 'void*' -> string. The LLVMContext (or any other context) can then be populated by the result of this parse. Each library can parse whatever argv it wants.
>  
> - I don't think we should keep piling on the interface design of the existing cl::opt stuff. I would make a clean break to a new header / library, and just build some code to map these into the cl::opt parsing code from the registry (we already do stuff like this for pass names). This will also help the two systems co-exist but stay separate during the (inevitably long) transition period.
>  
> - As a specific example of what I would change from the cl::opt stuff: I think we should make the required arguments just be required and positional, and the optional arguments use a fluent-style interface (yea, I know, but I think it works well here) rather than the weird variadic-ish thing...
>  
>  
> Once there is a patch that seems like a reasonable start at this, I'd actually like to get the new library added and just one pass (the one you're using seems great) converted. I'd then like to try a few experiments to see if we can improve some of the nitty-gritty details of the API before we roll it out widely.
>  
> Seem reasonable? I'm actually happy to help with some of the API experiments and the rollout here.
> -Chandler
>  
>  
> [1]: I'd actually like to understand why on some platforms the explicit global initialization routines are so much better than global constructors... But that's probably a topic for on off-list or IRC conversation. I don't want to derail this thread with that.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140827/3daea8d9/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: cl_opt-revised.diff
Type: application/octet-stream
Size: 11563 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140827/3daea8d9/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140827/3daea8d9/attachment-0001.html>