
<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 19, 2014, at 10:24 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=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Aug 19, 2014 at 10:10 PM, Pete Cooper <span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@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=""><div class=""><blockquote type="cite" class="">On Aug 19, 2014, at 6:45 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank" class="">chandlerc@google.com</a>> wrote:</blockquote>

</div><div class=""><br class=""></div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra">FWIW, I largely agree with Rafael's position here, at least in the near term.</div><div class="gmail_extra"><br class=""></div><div class="gmail_extra">

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.</div>

</div></div></blockquote></div>I actually disagree with this for two reasons.<div class=""><br class=""></div><div class="">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.</div>

</blockquote><div class=""><br class=""></div><div class="">No one is suggesting otherwise that I have seen? At least, my interpretation (perhaps incorrect, I've not had time to read all of this thread in 100% detail) was that the removal of static initializers wouldn't require changing every cl::opt variable.</div></div></div></div></div></blockquote><div><br class=""></div><div>I think I may be misunderstanding you here. If I understand Rafael correctly he wants all the option storage to be done using the cl::opt external storage capabilities, so that the option values are stored globally.</div><div><br class=""></div><div>My original proposal was to leave the storage in cl::opt, but to move the cl::opt to being owned, and have a keyed lookup. My plan was geared around being able to change one cl::opt at a time, but ultimately to get rid of the static initializers you do need to change every cl::opt variable so that they aren’t global.</div><div><br class=""></div><div>As part of this work I did want to eliminate the global storage for all these options in favor of having the data stored in the passes. It seems that this last is the contentious part, which is what Pete is talking about WRT the use of globals.</div><div><br class=""></div><div>I’m also not sure how Rafael’s proposal will work with eliminating static initializers for cl::opts with class or list typed data.</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote">

<div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class=""><br class=""></div><div class="">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. </div>

<div class=""><br class=""></div><div class="">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.</div></blockquote></div>

<br class="">Sure, you're arguing against the core design of cl::opt. However, we have it, and it wasn't an accident or for lack of other patterns that we chose it.</div><div class="gmail_extra"><br class=""></div><div class="gmail_extra">

For example, we don't require all constants to be per-pass, and instead have per-file constants. Rafael is suggesting that one use case for cl::opt global variables is, in essence, a constant that is somewhat easier for a developer of LLVM (*not* a user) to change during debugging and active development. I don't think the desire for convenience and only supporting the default value in production contexts are completely invalid.</div></div></div></blockquote><div><br class=""></div><div>I don’t think that my proposal adversely impacts the ability for a developer of LLVM to change the value of an option during debugging and development. If anything it makes this easier because it provides a way to do so without modifying source code (while not preventing you from doing it by modifying source code).</div><br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class="">

<div class="gmail_extra"><br class=""></div><div class="gmail_extra">Once you factor those in, we have a tradeoff. Historically, the tradeoff was made in the direction of convenience, relying on a very narrow interpretation of the use cases. It isn't clear to me that we should, today, make a different tradeoff. It certainly doesn't make sense why you would gate removing static initializers (a clear win) on forcing a change on a core design pattern within LLVM which not all of the developers are really supportive of (at this point).</div>

</div>

</div></blockquote><br class=""></div><div>I guess my point here is that there doesn’t need to be a tradeoff. My proposal doesn’t adversely impact convenience, and supports a wider use case.</div><div><br class=""></div><div>I do agree that we shouldn’t gate removing the static initializers on removing the globals, but I also don’t think this is really forcing a core design pattern change. If we can’t come to an agreement on the global data we can shelve the conversation.</div><br class=""><div class="">-Chris</div></body></html>