<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Thanks Chandler. I would very much appreciate having your feedback on this.<div class=""><br class=""></div><div class=""><div><blockquote type="cite" class=""><div class="">On Aug 20, 2014, at 11:18 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" class="">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">I'll try to provide some feedback on this patch assuming that we want to try to completely move away from global variables in a single step (something that I do generally support, although I don't understand why a more incremental approach is unacceptable -- it seems like a bit more work over all but a somewhat smoother transition for the community).</div></div></blockquote><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="">
<br class=""></div><div class="">I feel like the APIs you're ending up with are bending around backwards to achieve things that it isn't even clear are valuable:</div><div class=""><br class=""></div><div class="">1) They seem to be trying to working very hard to behave similarly to the existing cl::opt interfaces. If we're so radically changing the core design of cl::opt, different interfaces might be in order.</div></div></div></blockquote><div><br class=""></div><div>I wouldn’t say the APIs are working hard to behave similarly to the existing cl::opt interfaces as much as I designed this patch to provide incremental improvements to cl::opt, and changing the API was not on the table for the first change.</div><div><br class=""></div><div>The primary focus of this change is to provide a new way of defining existing cl::opt objects such that they are not globally scoped. This change should not preclude us from coming up with a new better API for options, but that should be a separate discussion from using the existing API while avoiding some of the less desirable side-effects.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">
<div class=""><br class=""></div><div class="">2) Perhaps this was explained up-thread (I'm going to try to read most of the older posts, but I'm still catching up on this thread) but if we're going this direction I would really expect the options to be per-LLVMContext... I feel like these are just "lazy globals with private storage" which don't seem all that conceptually better than globals. They've just engineered around the technical problems caused.</div></div></div></blockquote><div><br class=""></div><div>Earlier in the thread we’ve danced around this. My original proposal was to eventually create an option store object that would be passed into the context, but would be independent of it. The reason for not putting it in the context is because largest consumers of command line options are passes, and we already have use cases where we re-use pass managers with different contexts. Due to some early feedback on the thread I thought it would be best to shelve that conversation for a later date, but I’d love to know what feedback you have in that area.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">
<div class=""><br class=""></div><div class="">3) I really dislike the whole INITIALIZE_PASS_{BEGIN,END} pattern. I'm not sure if there is any realistic alternative, but I'm expecting to essentially have to rework every single one of these for the new pass manager. =[</div></div></div></blockquote><div><br class=""></div><div>I agree the INITIALIZE_PASS macros are nasty, but fundamentally I think you either need to be okay with global initializers (which we’re not), or you need to have some form of an explicit initialization.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">
<div class=""><br class=""></div><div class=""><br class=""></div><div class="">I'm going to try to read Sean's proposal, but I'm suspect it is essentially a strongly typed option registry inside the LLVMContext? With some way to collect the options first and then to parse flags? That kind of approach seems more likely to really solve this issue once and for all and be an excellent, and scalable long-term solution... but as I said, I'm still catching up on the thread. I'll try to post more coherent thoughts when I've had time to read the other posts fully and skim through the other proposals.</div></div></div></blockquote><div><br class=""></div><div>I don’t disagree that there are merits to Sean’s proposal, but there are also issues. I’d be more than happy to debate this in more detail, but I would actually ask that we set it aside for now. I’d really prefer if we didn’t gate removing static initializers and global option storage, which I think we can both agree are good things, on a larger change to a core design pattern within LLVM.</div><div><br class=""></div><div>Thanks,</div><div>-Chris</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">
<div class=""><br class=""></div></div><div class="gmail_extra"><br class=""><br class=""><div class="gmail_quote">On Wed, Aug 20, 2014 at 12:45 PM, Chris Bieneman <span dir="ltr" class=""><<a href="mailto:cbieneman@apple.com" target="_blank" class="">cbieneman@apple.com</a>></span> wrote:<br class="">
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">I think Rafael’s suggestion of using the address of a global as the key to the option store gives us the ability to accomplish all our goals without introducing a string keyed option store.<br class="">
<br class="">
I’ve updated my patches from the original proposal to reflect those changes.<br class="">
<br class="">
Does this look reasonable?<br class="">
<br class="">
</div>Thanks,<br class="">
-Chris<br class="">
<br class=""><br class="">
> On Aug 20, 2014, at 12:41 PM, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com" class="">rafael.espindola@gmail.com</a>> wrote:<br class="">
><br class="">
>>> Pass the passinfo to the pass constructor maybe?<br class="">
>>><br class="">
>>> I still don't understand what the problem with the global is. It has<br class="">
>>> the same value for all users. As Chandler pointed out, having<br class="">
>><br class="">
>> Globals are bad for many reasons: namespace pollution, concurrency issues, lack of access control, etc. etc.. Some of them have been discussed in this thread. Perhaps it’s not a big concern for most of the LLVM users. But we have an unique environment where LLVM is shared by multiple clients, and where the concern around exploits are especially strong. So while eliminating globals is not strictly tied to the elimination of static initializers, it is still a strong goal towards providing a LLVM dylib.<br class="">
><br class="">
> Fair enough. I would still make at separate stage since how to remove<br class="">
> them is the part that is still a bit contentious. Note that from a<br class="">
> security point of view a well known string probably makes things worse<br class="">
> than the global option storage since the address of the storage is at<br class="">
> least randomized.<br class="">
><br class="">
> It seems that both mine and Peter's proposal would avoid the global<br class="">
> storage and not introduce well known string for setting the options.<br class="">
><br class="">
> Cheers,<br class="">
> Rafael<br class="">
><br class="">
> _______________________________________________<br class="">
> LLVM Developers mailing list<br class="">
> <a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
<br class="">
<br class="">_______________________________________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
<br class=""></blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>