[LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)
Eric Christopher
echristo at gmail.com
Wed Nov 19 16:49:52 PST 2014
On Wed Nov 19 2014 at 4:38:02 PM Bob Wilson <bob.wilson at apple.com> wrote:
> On Nov 19, 2014, at 3:28 PM, Eric Christopher <echristo at gmail.com> wrote:
>
> So my general concern here is that lots of command line options that don't
> need to be or shouldn't be IR level constructs become oddly named string
> options. It's bad enough that we're doing that with the target cpu and
> target feature string let alone the rest of it.
>
>
> I’m not sure whether you’re concerned about the “oddly named” part or the
> “shouldn’t be IR” part. If the latter, this is why I previously suggested
> that we stage this so that after this patch goes in (assuming that we can
> reach consensus on that), we should consider each option separately. If
> there are some that don’t belong in IR, then we certainly shouldn’t do
> that. We need review each of them carefully to make that decision.
>
>
I don't think nearly all of them need to be in the IR :)
How about the rest of my mail?
-eric
>
> If we're talking about things that end up changing the subtarget used we
> definitely need to make this part of the lookup key - at which point we're
> going to want a unified interface to all of them. I haven't applied your
> patch to see what the IR changes look like here.
>
> From looking at the patch it seems that the command line option for NaNs
> should be a general attribute on the function rather than a TargetMachine
> option anyhow, so it seems that should be migrated down. Whether or not it
> is exposed as an llvm attribute or a string based key value pair is an
> interesting discussion. Also, would we want to, instead, expose the backend
> options as clang IR level options? Seems to make sense from that
> perspective and then they can add the appropriate attributes to the
> functions themselves. (-mlongcalls for the long calls option for example).
>
> Thanks!
>
> -eric
>
> On Tue Nov 18 2014 at 12:26:49 PM Akira Hatanaka <ahatanak at gmail.com>
> wrote:
>
>> 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/20141120/187158d0/attachment.html>
More information about the llvm-dev
mailing list