Bob Wilson
2014-Nov-17 17:52 UTC
[LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)
> On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: > > +chrisb > >> On 2014-Nov-13, at 16:33, Akira Hatanaka <ahatanak at gmail.com> wrote: >> >> 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. >> >> http://llvm.org/bugs/show_bug.cgi?id=21471 >> >> 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: >> >> 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? >> 2. How to handle cases where two functions in a module have different sets of command line options? >> 3. Where should the command line options or module/function attributes be stored once they are read out from the IR? >> >> 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. > > I like this approach, since it means the frontend doesn't have to understand > options in order to pass them on to the backend.I agree. It’s not the approach that was I was expecting, but after reading the patches, I like it. If we can get consensus on the approach, I suggest that we stage the work to first adopt Akira’s patches and then we can incrementally convert various options over to use it. There may be complications for some options, so I’d like to see separate patches for each one (possibly with a few similar options combined in one patch).> > The static variables should be straightforward to migrate to an LLVMContext > once ParseCommandLineOptions stores things there instead of in globals. > >> diff --git a/lib/CodeGen/CodeGenOption.cpp b/lib/CodeGen/CodeGenOption.cpp >> new file mode 100644 >> index 0000000..2d74c2f >> --- /dev/null >> +++ b/lib/CodeGen/CodeGenOption.cpp >> @@ -0,0 +1,59 @@ >> +//===- CodeGen/CodeGenOptions.cpp - Code-gen option. --*- C++ -*-===// >> +// >> +// The LLVM Compiler Infrastructure >> +// >> +// This file is distributed under the University of Illinois Open Source >> +// License. See LICENSE.TXT for details. >> +// >> +//===----------------------------------------------------------------------===// >> +// >> +//===----------------------------------------------------------------------===// >> + >> +#include "llvm/CodeGen/CodeGenOption.h" >> +#include "llvm/IR/Attributes.h" >> +#include "llvm/IR/Module.h" >> + >> +using namespace llvm; >> + >> +static std::map<std::string, bool> FunctionBoolAttrs; >> +static std::map<std::string, bool> ModuleBoolAttrs; >> + > > @Chris, should these be using ManagedStatic? > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Chris Bieneman
2014-Nov-17 18:07 UTC
[LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)
> On Nov 17, 2014, at 9:52 AM, Bob Wilson <bob.wilson at apple.com> wrote: > >> >> On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote: >> >> +chrisb >> >>> On 2014-Nov-13, at 16:33, Akira Hatanaka <ahatanak at gmail.com> wrote: >>> >>> 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. >>> >>> http://llvm.org/bugs/show_bug.cgi?id=21471 >>> >>> 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: >>> >>> 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? >>> 2. How to handle cases where two functions in a module have different sets of command line options? >>> 3. Where should the command line options or module/function attributes be stored once they are read out from the IR? >>> >>> 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. >> >> I like this approach, since it means the frontend doesn't have to understand >> options in order to pass them on to the backend. > > I agree. It’s not the approach that was I was expecting, but after reading the patches, I like it. > > If we can get consensus on the approach, I suggest that we stage the work to first adopt Akira’s patches and then we can incrementally convert various options over to use it. There may be complications for some options, so I’d like to see separate patches for each one (possibly with a few similar options combined in one patch).I’ve been thinking about this over the weekend. If we’re going to take a route like this, it would be better if we marked the options using one of the cl::opt property things (i.e. cl::hidden) rather than using a subclass of cl::opt. That also means moving the handling code either into the parser class, or (my preference) into cl::ParseCommandLine. Setting a property on the option will be easier to migrate to the new API. I also think that we need to get rid of the global storage somehow. It feels really nasty storing all the attributes globally when they really should probably be owned by the context or module. At the very least they should be ManagedStatic so that we avoid the static initializers. -Chris> >> >> The static variables should be straightforward to migrate to an LLVMContext >> once ParseCommandLineOptions stores things there instead of in globals. >> >>> diff --git a/lib/CodeGen/CodeGenOption.cpp b/lib/CodeGen/CodeGenOption.cpp >>> new file mode 100644 >>> index 0000000..2d74c2f >>> --- /dev/null >>> +++ b/lib/CodeGen/CodeGenOption.cpp >>> @@ -0,0 +1,59 @@ >>> +//===- CodeGen/CodeGenOptions.cpp - Code-gen option. --*- C++ -*-===// >>> +// >>> +// The LLVM Compiler Infrastructure >>> +// >>> +// This file is distributed under the University of Illinois Open Source >>> +// License. See LICENSE.TXT for details. >>> +// >>> +//===----------------------------------------------------------------------===// >>> +// >>> +//===----------------------------------------------------------------------===// >>> + >>> +#include "llvm/CodeGen/CodeGenOption.h" >>> +#include "llvm/IR/Attributes.h" >>> +#include "llvm/IR/Module.h" >>> + >>> +using namespace llvm; >>> + >>> +static std::map<std::string, bool> FunctionBoolAttrs; >>> +static std::map<std::string, bool> ModuleBoolAttrs; >>> + >> >> @Chris, should these be using ManagedStatic? >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev> > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu> http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141117/4e9aa206/attachment.html>
Eric Christopher
2014-Nov-17 19:40 UTC
[LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)
On Mon Nov 17 2014 at 9:56:40 AM Bob Wilson <bob.wilson at apple.com> wrote:> > > On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith < > dexonsmith at apple.com> wrote: > > > > +chrisb > > > >> On 2014-Nov-13, at 16:33, Akira Hatanaka <ahatanak at gmail.com> wrote: > >> > >> 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. > >> > >> http://llvm.org/bugs/show_bug.cgi?id=21471 > >> > >> 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: > >> > >> 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? > >> 2. How to handle cases where two functions in a module have different > sets of command line options? > >> 3. Where should the command line options or module/function attributes > be stored once they are read out from the IR? > >> > >> 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. > > > > I like this approach, since it means the frontend doesn't have to > understand > > options in order to pass them on to the backend. > > I agree. It’s not the approach that was I was expecting, but after reading > the patches, I like it. > > If we can get consensus on the approach, I suggest that we stage the work > to first adopt Akira’s patches and then we can incrementally convert > various options over to use it. There may be complications for some > options, so I’d like to see separate patches for each one (possibly with a > few similar options combined in one patch). > >I'm not sold quite yet :) Akira: Can you show an example of how you expect this to work in the IR and how the backend is going to process? Thanks. -eric> > > > The static variables should be straightforward to migrate to an > LLVMContext > > once ParseCommandLineOptions stores things there instead of in globals. > > > >> diff --git a/lib/CodeGen/CodeGenOption.cpp b/lib/CodeGen/CodeGenOption. > cpp > >> new file mode 100644 > >> index 0000000..2d74c2f > >> --- /dev/null > >> +++ b/lib/CodeGen/CodeGenOption.cpp > >> @@ -0,0 +1,59 @@ > >> +//===- CodeGen/CodeGenOptions.cpp - Code-gen option. --*- > C++ -*-===// > >> +// > >> +// The LLVM Compiler Infrastructure > >> +// > >> +// This file is distributed under the University of Illinois Open > Source > >> +// License. See LICENSE.TXT for details. > >> +// > >> +//===------------------------------------------------------ > ----------------===// > >> +// > >> +//===------------------------------------------------------ > ----------------===// > >> + > >> +#include "llvm/CodeGen/CodeGenOption.h" > >> +#include "llvm/IR/Attributes.h" > >> +#include "llvm/IR/Module.h" > >> + > >> +using namespace llvm; > >> + > >> +static std::map<std::string, bool> FunctionBoolAttrs; > >> +static std::map<std::string, bool> ModuleBoolAttrs; > >> + > > > > @Chris, should these be using ManagedStatic? > > > > _______________________________________________ > > LLVM Developers mailing list > > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141117/e79d3bd7/attachment.html>
Akira Hatanaka
2014-Nov-18 20:26 UTC
[LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)
Updated patch is attached. Note this is just a work-in-progress patch and I plan to address the feedback comments later if this patch is in the right direction. This is how the command line options are parsed and used by the backend passes: 1. Tools such as clang and llc call cl::ParseCommandLineOptions. Any of the options that should be written to the bitcode as function or module attributes (these options are declared as CodeGenOpt, which is a subclass of cl::opt) are saved to Function or ModuleIntAttrs in CodeGenOptions.cpp. 2. setFunctionAttribute is called to move the function and module options saved in step 1 to the bitcode. Also, options stored in TargetOptions are moved to the bitcode too. Pre-existing attributes in the bitcode are overwritten by the options in the command line. 3. The backend passes and subtarget classes call getFunctionAttribute to read the attributes stored to the bitcode in step 2. Function::hasFnAttribute can be called directly (for example, NoNaNsFPMath in the patch), if the default value is known to be "false". I also made the following changes in the patch: 1. In the constructor of MachineFunction, call the version of getSubtargetImpl that takes a Function parameter, so that it gets the Subtarget specific to the Function being compiled. 2. Change ARMTargetMachine::SubtargetMap to be a DenseMap<Function*, unique_ptr<ARMSubtarget>>. This is just a temporary change to ease debugging and should be reverted to a StringMap or changed to another type of map later. On Mon, Nov 17, 2014 at 11:40 AM, Eric Christopher <echristo at gmail.com> wrote:> > > On Mon Nov 17 2014 at 9:56:40 AM Bob Wilson <bob.wilson at apple.com> wrote: > >> >> > On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith < >> dexonsmith at apple.com> wrote: >> > >> > +chrisb >> > >> >> On 2014-Nov-13, at 16:33, Akira Hatanaka <ahatanak at gmail.com> wrote: >> >> >> >> 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. >> >> >> >> http://llvm.org/bugs/show_bug.cgi?id=21471 >> >> >> >> 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: >> >> >> >> 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? >> >> 2. How to handle cases where two functions in a module have different >> sets of command line options? >> >> 3. Where should the command line options or module/function attributes >> be stored once they are read out from the IR? >> >> >> >> 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. >> > >> > I like this approach, since it means the frontend doesn't have to >> understand >> > options in order to pass them on to the backend. >> >> I agree. It’s not the approach that was I was expecting, but after >> reading the patches, I like it. >> >> If we can get consensus on the approach, I suggest that we stage the work >> to first adopt Akira’s patches and then we can incrementally convert >> various options over to use it. There may be complications for some >> options, so I’d like to see separate patches for each one (possibly with a >> few similar options combined in one patch). >> >> > I'm not sold quite yet :) > > Akira: Can you show an example of how you expect this to work in the IR > and how the backend is going to process? > > Thanks. > > -eric > > >> > >> > The static variables should be straightforward to migrate to an >> LLVMContext >> > once ParseCommandLineOptions stores things there instead of in globals. >> > >> >> diff --git a/lib/CodeGen/CodeGenOption.cpp >> b/lib/CodeGen/CodeGenOption.cpp >> >> new file mode 100644 >> >> index 0000000..2d74c2f >> >> --- /dev/null >> >> +++ b/lib/CodeGen/CodeGenOption.cpp >> >> @@ -0,0 +1,59 @@ >> >> +//===- CodeGen/CodeGenOptions.cpp - Code-gen option. --*- >> C++ -*-===// >> >> +// >> >> +// The LLVM Compiler Infrastructure >> >> +// >> >> +// This file is distributed under the University of Illinois Open >> Source >> >> +// License. See LICENSE.TXT for details. >> >> +// >> >> +//===------------------------------------------------------ >> ----------------===// >> >> +// >> >> +//===------------------------------------------------------ >> ----------------===// >> >> + >> >> +#include "llvm/CodeGen/CodeGenOption.h" >> >> +#include "llvm/IR/Attributes.h" >> >> +#include "llvm/IR/Module.h" >> >> + >> >> +using namespace llvm; >> >> + >> >> +static std::map<std::string, bool> FunctionBoolAttrs; >> >> +static std::map<std::string, bool> ModuleBoolAttrs; >> >> + >> > >> > @Chris, should these be using ManagedStatic? >> > >> > _______________________________________________ >> > LLVM Developers mailing list >> > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141118/c7248c1e/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: llvm-pr21471.patch Type: application/octet-stream Size: 20613 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141118/c7248c1e/attachment.obj> -------------- next part -------------- A non-text attachment was scrubbed... Name: clang-pr21471.patch Type: application/octet-stream Size: 2930 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20141118/c7248c1e/attachment-0001.obj>