<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 Nov 17, 2014, at 10:17 AM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" class="">ahatanak@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote">On Fri, Nov 14, 2014 at 3:24 PM, Pete Cooper<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><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 style="word-wrap: break-word;" class=""><br class=""><div class=""><span class=""><blockquote type="cite" class=""><div class="">On Nov 14, 2014, at 2:57 PM, Chris Bieneman <<a href="mailto:cbieneman@apple.com" target="_blank" class="">cbieneman@apple.com</a>> wrote:</div><br class=""><div class=""><span 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; float: none; display: inline !important;" class="">There are parts of this proposal that I really like, and there are others that I think are actually at opposition to the work we’re trying to do with cl::opt.</span><div 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;" class=""><br class=""><div class=""><blockquote type="cite" class=""><div class="">On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" target="_blank" class="">dexonsmith@apple.com</a>> wrote:</div><br class=""><div class=""><span 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; float: none; display: inline !important;" class="">+chrisb</span><br 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;" class=""><br 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;" class=""><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;" class="">On 2014-Nov-13, at 16:33, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank" class="">ahatanak@gmail.com</a>> wrote:<br class=""><br class="">I'm working on fixing PR21471, which is about embedding codegen command line options into the bitcode as function or module-level attributes so that they don't get ignored when doing LTO.<br class=""><br class=""><a href="http://llvm.org/bugs/show_bug.cgi?id=21471" target="_blank" class="">http://llvm.org/bugs/show_bug.cgi?id=21471</a><br class=""><br class="">I have an initial patch (attached to this email) which enables clang/llvm to recognize one command line option, write it to the IR, and read it out in a backend pass. I'm looking to get feedback from the community on whether I'm headed in the right direction or whether there are alternate ideas before I go all the way on fixing the PR. Specifically, I'd like to know the answers to the following questions:<br class=""><br class="">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 class=""><br class=""></div><div class="">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></div></div></blockquote></span>Ideally this will be a small set as it could get expensive to represent otherwise. I’ll get to why later.</div><div class=""><br class=""></div><div class="">So with LTO we already have issues where modules have different metadata. I’m not sure, but we might also have issues with weak functions and different attribute sets.</div><div class=""><br class=""></div><div class="">We need to work out whether the option a function was compiled with is interesting enough to be stored on that function, even if its the default. For example, lets say I tag a function with ‘loop-unroll-threshold=100’, I would expect that to override the one given on the command line, but perhaps others would want the command line to always win. </div><div class=""><br class=""></div></div></blockquote><div class=""><br class=""></div><div class="">There is a text file attached to PR21471 which lists the command line options that are generated by clang and are necessary for code generation as of r217366. Many of them are relevant as losing them during LTO can result in incorrect code generation.</div></div></div></div></div></blockquote>Thanks. Thats a really useful document!</div><div><br class=""></div><div>So to revise what I said before, it doesn’t look like you are actually configuring individual passes at all. Certainly not initially. From the list it looks like you are more likely to, for example, choose whether to run the LoopUnrollPass at all, that to configure the internal loop unroll threshold.</div><div><br class=""></div><div>I think its good to start with these as hopefully the scope is smaller. I first thought you were going to need to save the defaults for all options in all passes which would have been a significant amount of work. Storing the options you need to configure the pass manager is much more manageable.</div><div><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> </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 style="word-wrap: break-word;" class=""><div class=""></div><div class="">Then there’s the issue of whether a default is interesting or not. For example, the default loop unroll threshold is 150. We probably want to tag all functions with that threshold as how to we know that the default will stay the same in a later LLVM. Or you could save the fact that something is a default. So for example, store ‘unroll-threadhold=default(150)’ as then you can either:</div><div class="">- Always choose 150 for this function, because thats what it was tagged with</div><div class="">- Always choose the default, so if we change ToT to default 200, you choose 200.</div><div class=""><br class=""></div><div class="">Now if you have to store all ‘interesting’ options, the set of things you store could start to get quite large quite quickly.</div><div class=""><span class=""><blockquote type="cite" class=""><div class=""><div 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;" class=""><div class=""><br class=""></div></div></div></blockquote></span></div></div></blockquote><div class="">I didn't think about storing the default values, but if the set of "interesting options" is small, we can still store them in the bitcode.</div></div></div></div></div></blockquote>Yeah, I think it will be. In particular, you can save space if any of the options are deemed important enough to give an enum name instead of a string. For example, I don’t expect anyone to argue that we are going to have OptLevel forever so you could make it an IntAttribute and store it efficiently. </div><div><br class=""></div><div>Now perhaps SizeLevel isn’t as likely to live forever (just an example, I have no idea), we could store it as a StringAttribute: “SizeLevel”=“1”, although since its only the name changing which would be a problem, I would introduce a new type of attribute which has the value as an integer. So “SizeLevel”=1. Then you don’t need to actually parse it via ParseCommandLineOptions and you can more efficiently store it in the bitcode.</div><div><br class=""></div><div>Thanks,</div><div>Pete<br class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""> <br class=""></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 style="word-wrap: break-word;" class=""><div class=""><span class=""><blockquote type="cite" class=""><div class=""><div 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;" class=""><div class=""><blockquote type="cite" class=""><div class=""><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;" class="">2. How to handle cases where two functions in a module have different sets of command line options?<br class=""></blockquote></div></blockquote></div></div></div></blockquote></span>I would store them in the attributes set. I don’t think there’s anywhere else you can do this right now. Attributes or metadata. I would say attributes because then you can make them “option”=“value” (i.e., StringAttribute) and you don’t need to worry about anyone knowing about the names or not.<span class=""><br class=""><blockquote type="cite" class=""><div class=""><div 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;" class=""><div class=""><div class=""><br class=""></div><div class="">Today I don’t believe we have this ability.</div><br class=""><blockquote type="cite" class=""><div class=""><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;" class="">3. Where should the command line options or module/function attributes be stored once they are read out from the IR?</blockquote></div></blockquote></div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div 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;" class=""><div class=""><div class=""><br class=""></div>My suggestion would be the OptionStore that I proposed here: <a href="http://reviews.llvm.org/D6207" target="_blank" class="">http://reviews.llvm.org/D6207</a></div></div></div></blockquote></span>I don’t think the OptionStore will work either, unless you put an OptionStore on the Function which I don’t think will be feasible.</div><div class=""><br class=""></div><div class="">Instead I think you need either the function pass manager (perhaps LPPassManager, BB PM, etc too) to parse the options from the function once before it runs all the passes. Or, for each pass with an option you care about, move the storage of that option itself to the pass. This is similar to what Chris is doing with static initializers. So I can imagine pass initialization looking something like</div><div class=""><br class=""></div><div class="">class LoopUnrollPass {</div><div class=""> <span class="Apple-converted-space"> </span>unsigned Threshold = ...</div><div class="">doIniit… {</div><div class=""> <span class="Apple-converted-space"> </span>if (cl opt has value)</div><div class=""> <span class="Apple-converted-space"> </span>Threshold = ‘cl opt value’</div><div class=""> <span class="Apple-converted-space"> </span>if (function.getattribute(‘unroll-threshold’)</div><div class=""> <span class="Apple-converted-space"> </span>Threadhold = function.getAttributeAsInteger(‘unroll-threshold’)</div><div class="">}</div><div class=""><br class=""></div><div class="">For performance reasons, I would actually add a new type of Attribute for a string key and integer value as then you don’t actually need to do any parsing in the new function.getAttributeAsInteger function I introduced here.</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Pete<br class=""><blockquote type="cite" class=""><div class=""><div class=""><div class=""><div 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;" class=""><div class=""><br class=""><blockquote type="cite" class=""><div class=""><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;" class=""><br class="">The short description of the approach I took in my patch is that command line options that are important to codegen are collected by cl::ParseCommandLineOptions, written to the bitcode as function or module attributes, and read out directly by the optimization passes that need them. cl::opt options are replaced with CodeGenOpt options (subclass of cl::opt), which are needed only to parse the command line and provide the default value when the corresponding options are not in the bitcode.<br class=""></blockquote><br 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;" class=""><span 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; float: none; display: inline !important;" class="">I like this approach, since it means the frontend doesn't have to understand</span><br 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;" class=""><span 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; float: none; display: inline !important;" class="">options in order to pass them on to the backend.</span><br 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;" class=""><br 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;" class=""><span 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; float: none; display: inline !important;" class="">The static variables should be straightforward to migrate to an LLVMContext</span><br 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;" class=""><span 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; float: none; display: inline !important;" class="">once ParseCommandLineOptions stores things there instead of in globals.</span><br 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;" class=""></div></blockquote><div class=""><br class=""></div>I also think that the OptionStore in conjunction with the OptionRegistry (rather than any of the cl APIs) should have all the parsing code. In fact, you shouldn’t have to call ParseCommandLineOptions, we could make encoding and decoding the stored options associated with a module part of loading and storing the module.</div><div class=""><br class=""><blockquote type="cite" class=""><div class=""><br 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;" class=""><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;" class="">diff --git a/lib/CodeGen/CodeGenOption.cpp b/lib/CodeGen/CodeGenOption.cpp<br class="">new file mode 100644<br class="">index 0000000..2d74c2f<br class="">--- /dev/null<br class="">+++ b/lib/CodeGen/CodeGenOption.cpp<br class="">@@ -0,0 +1,59 @@<br class="">+//===- CodeGen/CodeGenOptions.cpp - Code-gen option. --*- C++ -*-===//<br class="">+//<br class="">+// The LLVM Compiler Infrastructure<br class="">+//<br class="">+// This file is distributed under the University of Illinois Open Source<br class="">+// License. See LICENSE.TXT for details.<br class="">+//<br class="">+//===----------------------------------------------------------------------===//<br class="">+//<br class="">+//===----------------------------------------------------------------------===//<br class="">+<br class="">+#include "llvm/CodeGen/CodeGenOption.h"<br class="">+#include "llvm/IR/Attributes.h"<br class="">+#include "llvm/IR/Module.h"<br class="">+<br class="">+using namespace llvm;<br class="">+<br class="">+static std::map<std::string, bool> FunctionBoolAttrs;<br class="">+static std::map<std::string, bool> ModuleBoolAttrs;<br class="">+<br class=""></blockquote><br 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;" class=""><span 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; float: none; display: inline !important;" class="">@Chris, should these be using ManagedStatic?</span></div></blockquote><br class=""></div><div class="">I’d much rather they just weren’t static at all. Using globals to store state that inherently isn’t global just feels wrong.</div><div class=""><br class=""></div><div class="">-Chris</div><br class=""></div></div></div><span class=""><span 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; float: none; display: inline !important;" class="">_______________________________________________</span><br 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;" class=""><span 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; float: none; display: inline !important;" class="">LLVM Developers mailing list</span><br 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;" class=""><a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank" 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;" class="">LLVMdev@cs.uiuc.edu</a><span 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; float: none; display: inline !important;" class=""><span class=""> </span> </span><a href="http://llvm.cs.uiuc.edu/" target="_blank" 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;" class="">http://llvm.cs.uiuc.edu</a><br 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;" class=""><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" 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;" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></span></div></blockquote></div><br class=""></div><br class="">_______________________________________________<br class="">LLVM Developers mailing list<br class=""><a href="mailto:LLVMdev@cs.uiuc.edu" target="_blank" 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></blockquote></div></div></div></div></blockquote></div><br class=""></body></html>