<div dir="ltr">Reply to list as I had originally intended...<br><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">Justin Holewinski</b> <span dir="ltr"><<a href="mailto:justin.holewinski@gmail.com">justin.holewinski@gmail.com</a>></span><br>
Date: Wed, Feb 13, 2013 at 4:23 PM<br>Subject: Re: [LLVMdev] Overhauling the command-line processing/codegen options code<br>To: Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>><br><br><br>
<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote"><div class="im">On Wed, Feb 13, 2013 at 3:53 PM, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com" target="_blank">clattner@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>On Feb 13, 2013, at 11:37 AM, Justin Holewinski <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>> wrote:<br>


> Is anyone currently working on overhauling the command-line processing code?<br>
<br>
</div>Bill's attributes work is quite related, it will hopefully define away a number of the TargetOptions concepts, replacing them with per-function attributes.  It should also subsume a number of target-independent concepts like "omit frame pointers" and other codegen flags.<br>


<div><br>
> We're currently having some design issues with this component, and I'd like to start a larger conversation on it.  For context, I am talking from an "LLVM as a library" perspective<br>
<br>
</div>My favorite perspective :)<br>
<div><br>
>  In a nut-shell, the problems we are facing are as follows:<br>
><br>
</div>> 1. As per bug 11944, the command-line processing logic accounts for a non-trivial amount of global constructors and leads to memory that is allocated when the containing library is loaded, and not freed until the containing library is unloaded.  Granted, this is not a lot of data…<br>


<br>
Yep, this sucks.  It has been a long-known problem.  In my ideal world, the only cl::opt's that would exist would be in leaf tools (e.g. in tools/opt, but not in optimization passes themselves).  There is some major design work required to make this happen though, and I'm not aware of anyone tackling it.<br>

</blockquote><div><br></div></div><div>Right, the sheer amount of work here gives me night terrors... but fortunately my current problem isn't really related to static data as much as it is point (2) below. :)  Though on the topic of static data, ManagedStatic<> cleanup is another topic that bites me, but that is for another day.</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> 2. Command-line options are currently used for codegen options in inherently non-thread-safe ways.<br>
<br>
</div>Hopefully Bill's work will help address the majority of this.<br></blockquote><div><br></div></div><div>Bill's work does seem very relevant here, but I'm unsure if module attributes is really the right approach here.  To me, it makes sense to keep code generation options separate from the IR itself.  If we use attributes, that means the tools would actually modify the IR (in the form of adding attributes) before running it through the pass pipeline.</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> (1) is mostly self-explanatory and has a bug about it, but (2) requires a bit more explanation.  Take for example the "-unroll-threshold" option in LoopUnroll.  If I'm setting up a pass pipeline programmatically, I add the LoopUnroll pass to the PassManager, but I cannot set the unroll threshold without calling cl::ParseCommandLineOptions() [or perhaps some ugly hacks that involve getting at and modifying the global state].  In addition to being awkward, this is not thread safe!  I cannot run two PassManager pipelines concurrently with different unroll threshold values.  In this case, I am singling out the LoopUnroll pass, but this design is very prevalent within the LLVM analysis/transformation/codegen infrastructure.  This has no effect on users of opt/llc as tools, but library users can be greatly affected by this.<br>


<br>
</div>This is a very interesting one that is orthogonal to Bill's work.  The preferred approach for this sort of thing is to change the LoopUnroll pass to take the unroll threshold as a constructor argument or a struct that wraps up all of the configuration settings.  When the pass is *default* constructed (e.g. from opt -loop-unroll), it is acceptable to have the default ctor read cl::opt variables, but the optimization pipeline should not depend on cl::opts to configure the pass.<br>


<br>
There is currently some gray area here for debug settings, and for things that are being staged in but are not on by default, but the default optimization pipeline should not be looking to cl::opt's for their settings (in an ideal world).<br>

</blockquote><div><br></div></div><div>I agree that this is a much cleaner solution for statically-linked passes, but how do you handle options for passes loaded at run-time?  One of the advantages of the current command-line option approach is that I can define options and have opt/llc accept them from modules linked in with "-load".  With struct-based options, I would have no way of having my library dynamically load another containing passes, and set options on those passes (unless I used some common base class that allowed me to set options).  Or perhaps a trivial solution would be to just add a new virtual method to all passes that allows clients to pass arbitrary options, like a "virtual void setOption(StringRef Option, StringRef Value) {}".  Though this use-case seems very rare...</div>
<div class="im">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><br>
> Requirements<br>
><br>
> - Make it easy for passes to declare arbitrary options, just like they do now<br>
> - Let different pass pipelines have different sets of option values<br>
> - Keep the option registry dynamic, so plugins loaded with "-load" can register new options<br>
</div>> - Let option values be parseable from the command-line (for opt, llc, …)<br>
<br>
All good requirements.<br>
<div><br>
> Implementation<br>
><br>
> As a first design draft, I propose that cl::opt and friends be extended to support argument storage in an LLVMContext.  Instead of storing the value directly in a cl::opt instance or specifying a global variable, this new storage would utilize a StringMap stored in an LLVMContext.  As a consequence, parsing would be delayed until the option is read.<br>


<br>
</div>I'd really like to avoid going here.  With the direction sketched out above, is enough of your problem solved?  Taking loop unroll as an example, I'd much rather you refactor the code to have its constructor take a configuration struct setting its various settings, than have the various settings pulled out to llvmcontext.<br>

</blockquote><div><br></div></div><div>I believe it would solve the issues I have, yes.  That would mean having two versions of every createXXXPass() function, one for default parameters and one taking an options struct.  If we ignore the library-loading-library case, I could see this working.</div>

<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><font color="#888888"><br>
-Chris<br>
<br>
<br><span class="HOEnZb"><font color="#888888">
</font></span></font></span></blockquote></div><span class="HOEnZb"><font color="#888888"><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div>
</font></span></div></div>
</div><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div>
</div>