<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 4, 2013 at 1:39 PM, Puyan Lotfi <span dir="ltr"><<a href="mailto:plotfi@apple.com" target="_blank" class="cremed">plotfi@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":551" style="overflow:hidden">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.<br>
</div></blockquote><div><br></div><div>I don't think there is a common case where performance is a concern. Why?</div><div><br></div><div>1) We're parsing commandline flags in an inherently race-prone way, and for flags which are explicitly for debugging and custom manipulations only.</div>
<div>2) It can only happen once per LLVMContext spin up / tear down</div><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div id=":551" style="overflow:hidden">
<div class="im"><br>
> 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.<br>

><br>
<br>
</div>I think this could work. Do you think a persistent uniquing string pool (StringMap of char*’s) in LLVMContext would be in order?<br>
<div class="im"></div></div></blockquote></div><br>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.</div>
<div class="gmail_extra"><br></div><div class="gmail_extra">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.</div>
</div>