<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 19, 2014 at 6:06 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra">So I am deeply concerned about the direction this is taking. I'm trying to catch up on the thread, but I think Chris already highlighted my issue:</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_quote"><span>On Fri, Nov 14, 2014 at 4:57 PM, Chris Bieneman <span dir="ltr"><<a href="mailto:cbieneman@apple.com" target="_blank">cbieneman@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><span><blockquote type="cite"><div><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">1. How do we make sure we continue to be able to use the command line options we've been using for llc and other tools?</blockquote></div></blockquote><div><br></div></span><div>In discussions about the new cl::opt API I believe the general idea was that most of the options expressed using cl::opt are actually only relevant as debug options, so I think one big part of this work is really going to be identifying a subset of the current options which are actually relevant to expose in the IR.</div></div></blockquote><div><br></div></span><div>I think this is critical.</div><div><br></div><div>The whole idea of cl::opt API is for *debugging* options. IE, not supported, expected variations on how passes behave. Those should always be controlled (at the LLVM API layer) through constructors and parameters, not through a side-layer.</div><div><br></div></div></div></div></blockquote><div><br></div><div>Maybe there is some misunderstanding here. The patch I sent doesn't change llvm to control the behavior of passes through cl::opt options that are currently defined as static variables. The function attributes embedded in the IR control the behavior. The option variables (defined as CodeGenOpt, which is a subclass of cl:opt) have only two roles:</div><div><br></div><div>1. Enable parsing the option that appears in the command line.</div><div>2. Provide the default value of the option in case neither the bitcode nor command line had the attribute/option the pass is looking for.</div><div><br></div><div>It isn't absolutely necessary to keep those variables in the file, they can be safely moved to another file as long as there is a way to get the default value.<br></div><div><br></div><div>Also, I'm not sure customizing the behavior of passes through constructors is a good idea, Wouldn't that mean you have to instantiate the passes multiple times if there are functions that require different behaviors in the module?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div>There are parts of LLVM currently abusing the cl::opt mechanism to control fundamental functionality, but we should *absolutely* not bake any part of that or support for that into the IR! We should go find and fix those places to use reasonable APIs. Once we have that, I am very supportive of getting a good system for transmitting these options in bitcode and such in order to better support LTO. However, I think that in essentially every case there are going to be two options:</div><div><br></div><div>1) Turn these options into function attributes because they can reasonably live as function attributes and different variations can co-exist within a module.</div><div><br></div><div>2) Keep the options as module-level options, but insist that they match for all modules being merged in LTO.</div><div><br></div><div>3) (very rare) have clean, well specified merge semantics to merge different options from different modules in LTO. I think these only come up quite rarely. The only really good example I know of would be something like "library link dependencies" where it is a list that we clearly just take the union to merge.</div><span><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div><span><br><blockquote type="cite"><div><blockquote type="cite" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">3. Where should the command line options or module/function attributes be stored once they are read out from the IR?<br></blockquote></div></blockquote><div><br></div></span>My suggestion would be the OptionStore that I proposed here: <a href="http://reviews.llvm.org/D6207" target="_blank">http://reviews.llvm.org/D6207</a></div><div></div></blockquote></span></div><br>Not to knock the option store (i quite like it), but I think that should be reserved for the cl::opt-style (but with your new API which is way better) debugging options, and never touch the IR.</div></div>
<br>_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank">LLVMdev@cs.uiuc.edu</a>         <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
<br></blockquote></div><br></div></div>