[PATCH] Allow for the use of StringRef command line options instead of cl::opt<std::string>

[Chandler Carruth]

On Fri, Oct 4, 2013 at 1:39 PM, Puyan Lotfi <plotfi at apple.com> wrote:

> I like this solution but do you think there should be some way to tell the
> library (optional parameter, or a separate API function) to use the argv
> data directly instead of allocating memory on the context. Seems like it
> would be good to not hurt performance of the common case.
>

I don't think there is a common case where performance is a concern. Why?

1) We're parsing commandline flags in an inherently race-prone way, and for
flags which are explicitly for debugging and custom manipulations only.
2) It can only happen once per LLVMContext spin up / tear down

If these options really need to be controlled by library users in the
common case, they *need* to be lifted up to proper APIs on the passes or
PassManagerBuilder. The flags interface is for "opt", debugging,
experimentation, etc. Currently, the only users I know of who abuse this
are Clang itself for several backend options, but these are *bugs* that
need to be fixed, and so I'm not at all worried about their performance.


>
> > Without doing something to take ownership of the memory when parsing
> flags, this change will break a very large number of users of LLVM, so I
> don't think there is going to be a significantly simpler solution here.
> >
>
> I think this could work. Do you think a persistent uniquing string pool
> (StringMap of char*’s) in LLVMContext would be in order?
>

I don't think a StringMap makes much sense. There is no string key here. I
don't think you even need to do uniquing, because it just doesn't seem like
performance is going to matter here.

I would use a DenseMap<cl::opt<StringRef>*, const char*> or some such... I
wish we had a nice unique_ptr to store as the value type, but whatever.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131004/e8c769ab/attachment.html>