llvm dev - Nov 2014 - [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>

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.

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/20141119/d928cceb/attachment.html>

> 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.
> 
> 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
<mailto: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 <mailto:echristo at gmail.com>> wrote:
> 
> 
> On Mon Nov 17 2014 at 9:56:40 AM Bob Wilson <bob.wilson at apple.com
<mailto:bob.wilson at apple.com>> wrote:
> 
> > On Nov 14, 2014, at 2:44 PM, Duncan P. N. Exon Smith <dexonsmith at
apple.com <mailto:dexonsmith at apple.com>> wrote:
> >
> > +chrisb
> >
> >> On 2014-Nov-13, at 16:33, Akira Hatanaka <ahatanak at gmail.com
<mailto: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
<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 <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>
> 
> _______________________________________________
> 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/20141119/0231a8c1/attachment.html>

On Wed, 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.
>
> 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.
>
>
With the patches applied, the attributes clang embeds in the IR look like
this:

$ clang -cc1 ... -mllvm -arm-long-calls -mllvm -arm-reserve-r9
-menable-no-nans

attributes #0 = { nounwind readonly ssp "NoNaNsFPMath"
"arm-long-calls"="1"
"arm-reserve-r9"="1"
"less-precise-fpmad"="false"
"no-frame-pointer-elim"="true"
"no-frame-pointer-elim-non-leaf"
"no-infs-fp-math"="false"
"stack-protector-buffer-size"="8"
"target-cpu"="arm7tdmi"
"target-features"="-neon,-crypto,+apcs"
"unsafe-fp-math"="false"
"use-soft-float"="true" }

I'm guessing you want the front-end to convert command line options like
"arm-long-calls" or "arm-reserve-r9" to something that
doesn't look as odd?
> 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/20141119/9df38e59/attachment.html>