llvm dev - Nov 2014 - [LLVMdev] [RFC] Embedding command line options in bitcode (PR21471)

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.

Other possible ideas that I've discussed or thought about, but haven't
implemented:

1. Treat cl::opt options as overrides, possibly by setting a bit that
indicates the option has been specified in the command line.

2. Extend the idea proposed in the discussion on llvm-dev about removing
static initializer for command line options:

http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-August/075886.html

I looked at the code that was checked in to trunk and it seems to me that
it isn't possible to have storage for command line options on a
per-function storage basis yet, which I think is necessary if the functions
in a module have different sets of command line options.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141113/32604258/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: llvm-pr21471.patch
Type: application/octet-stream
Size: 10379 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141113/32604258/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-pr21471.patch
Type: application/octet-stream
Size: 844 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141113/32604258/attachment-0001.obj>

Hi Akira,

This is very closely related to the work I've been doing and so I care
quite a bit about it. I've implemented some of this - at least as far as
the global TargetMachine options in the current work for Subtarget code
generation - which is what some of this comes down to. I'll respond inline
here:

On Thu Nov 13 2014 at 4:35:08 PM 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.
>
Glad to see you working on it.

> 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?
>
>
Yes. These are some of the important questions. I think you're missing a
few things to take into consideration:

a) How does this change the pass manager? Some of the command line options
(many) change which passes are run when. It should be as simple as checking
the function attribute for each pass to decide when to run, but if one
invokes a chain then you might have other issues or if the command line
option invokes module/cgscc passes (see c below).

b) Right now I'm using a combination of stringified target-cpu and
target-features with each target's cpu specific attributes (i.e. something
like mips16) being a separate option that gets plugged into the subtarget -
if it controls the creation of a subtarget. I don't know that any do (at
least a quick glance didn't seem to say), but if a command line option
controls any of the initialization in the subtarget dependent features
it'll need to be part of the key to look up the subtarget. If it doesn't
then you'll just need to check the attribute, as you said, in the
pass/lowering/thingy.

c) Which command line options need to be part of this interface? I don't
necessarily think all of them should which could turn some of these into
subtarget features that can just be added on to the functions as they go.
If anything can't change between translation units then a module level flag
that errors on merge would be applicable. I'd prefer not to use module
level flags for things that can just be put on every function. The module
level flags could be read into a subtarget specific global target options
flag (ala soft-float).

d) linkonce_odr functions with different attributes

e) How to organize these so that it's easy for a particular target to know
what attributes it might put on a function or module?

Probably more :)

Other possible ideas that I've discussed or thought about, but
haven't
> implemented:
>
> 1. Treat cl::opt options as overrides, possibly by setting a bit that
> indicates the option has been specified in the command line.
>
> 2. Extend the idea proposed in the discussion on llvm-dev about removing
> static initializer for command line options:
>
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-August/075886.html
>
> I looked at the code that was checked in to trunk and it seems to me that
> it isn't possible to have storage for command line options on a
> per-function storage basis yet, which I think is necessary if the functions
> in a module have different sets of command line options.
>
>
I don't think either of these are a good idea.

Thoughts?

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141114/7c87d4e7/attachment.html>

+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.

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?

> On 2014-Nov-14, at 13:58, Eric Christopher <echristo at gmail.com>
wrote:
> 
> Hi Akira,
> 
> This is very closely related to the work I've been doing and so I care
quite a bit about it. I've implemented some of this - at least as far as the
global TargetMachine options in the current work for Subtarget code generation -
which is what some of this comes down to. I'll respond inline here:
> 
> On Thu Nov 13 2014 at 4:35:08 PM 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.
> 
> Glad to see you working on it.
>  
> 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?
> 
> 
> Yes. These are some of the important questions. I think you're missing
a few things to take into consideration:
> 
> a) How does this change the pass manager? Some of the command line options
(many) change which passes are run when.
Unfortunately the pass manager isn't a safe place to hack right now.
> It should be as simple as checking the function attribute for each pass to
decide when to run,
Right, these are easy.
> but if one invokes a chain then you might have other issues
Just for clarity, if an option changes the passes from:

    -instcombine

to:

    -instcombine -some-pass1 -some-pass2 -instcombine

then ideally, we'd attach an attribute to the functions affected by
that option, and `-some-pass1 -some-pass2 -instcombine` would be
skipped for functions without that attribute.

IIUC, you're saying that skipping the second -instcombine is hard,
which I agree with.

Agreed.  I'm comfortable leaving that to be solved later, once the
new pass manager is in place.  Thoughts?
> or if the command line option invokes module/cgscc passes (see c below).
>  
> b) Right now I'm using a combination of stringified target-cpu and
target-features with each target's cpu specific attributes (i.e. something
like mips16) being a separate option that gets plugged into the subtarget - if
it controls the creation of a subtarget. I don't know that any do (at least
a quick glance didn't seem to say), but if a command line option controls
any of the initialization in the subtarget dependent features it'll need to
be part of the key to look up the subtarget. If it doesn't then you'll
just need to check the attribute, as you said, in the pass/lowering/thingy.
> 
> c) Which command line options need to be part of this interface? I
don't necessarily think all of them should which could turn some of these
into subtarget features that can just be added on to the functions as they go.
Can you give an example of these?
> If anything can't change between translation units then a module level
flag that errors on merge would be applicable. I'd prefer not to use module
level flags for things that can just be put on every function. The module level
flags could be read into a subtarget specific global target options flag (ala
soft-float).
Agreed.
> d) linkonce_odr functions with different attributes
Doesn't this just get handled by lib/Linker?  I figure we keep the
attributes of the version of the function that "wins".
> 
> e) How to organize these so that it's easy for a particular target to
know what attributes it might put on a function or module?
> 
> Probably more :)
> 
> Other possible ideas that I've discussed or thought about, but
haven't implemented:
> 
> 1. Treat cl::opt options as overrides, possibly by setting a bit that
indicates the option has been specified in the command line.
> 
> 2. Extend the idea proposed in the discussion on llvm-dev about removing
static initializer for command line options:
> 
> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-August/075886.html
> 
> I looked at the code that was checked in to trunk and it seems to me that
it isn't possible to have storage for command line options on a per-function
storage basis yet, which I think is necessary if the functions in a module have
different sets of command line options.
> 
> 
> I don't think either of these are a good idea.
> 
> Thoughts?
> 
> -eric
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

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.
> 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?
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.
>> 2. How to handle cases where two functions in a module have different
sets of command line options?
Today I don’t believe we have this ability.
>> 3. Where should the command line options or module/function attributes
be stored once they are read out from the IR?
My suggestion would be the OptionStore that I proposed here:
http://reviews.llvm.org/D6207 <http://reviews.llvm.org/D6207>
>> 
>> 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.
> 
> The static variables should be straightforward to migrate to an LLVMContext
> once ParseCommandLineOptions stores things there instead of in globals.
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.
> 
>> 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?
I’d much rather they just weren’t static at all. Using globals to store state
that inherently isn’t global just feels wrong.

-Chris

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141114/2a9cd9be/attachment.html>

>
>
> a) How does this change the pass manager? Some of the command line options
> (many) change which passes are run when. It should be as simple as checking
> the function attribute for each pass to decide when to run, but if one
> invokes a chain then you might have other issues or if the command line
> option invokes module/cgscc passes (see c below).
>
>
Relatedly you might want to look at all of the flags passed via
-backend-args, i.e.  -enable-global-merge=false as a good first step for
things that will need to be handled this way. I.e. some optimization
options could be turned off because of known code problems with those
function (similarly opt levels etc).

Honestly not sure how important this use is though. LTO + different
optimization options for each module/function? Weird. But it's something to
keep in mind I guess.

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141115/a09e31b6/attachment.html>

On Fri, Nov 14, 2014 at 1:58 PM, Eric Christopher <echristo at gmail.com>
wrote:
> Hi Akira,
>
> This is very closely related to the work I've been doing and so I care
> quite a bit about it. I've implemented some of this - at least as far
as
> the global TargetMachine options in the current work for Subtarget code
> generation - which is what some of this comes down to. I'll respond
inline
> here:
>
> On Thu Nov 13 2014 at 4:35:08 PM 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.
>>
>
> Glad to see you working on it.
>
>
>> 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?
>>
>>
> Yes. These are some of the important questions. I think you're missing
a
> few things to take into consideration:
>
> a) How does this change the pass manager? Some of the command line options
> (many) change which passes are run when. It should be as simple as checking
> the function attribute for each pass to decide when to run, but if one
> invokes a chain then you might have other issues or if the command line
> option invokes module/cgscc passes (see c below).
>
>
b) Right now I'm using a combination of stringified target-cpu
and
> target-features with each target's cpu specific attributes (i.e.
something
> like mips16) being a separate option that gets plugged into the subtarget -
> if it controls the creation of a subtarget. I don't know that any do
(at
> least a quick glance didn't seem to say), but if a command line option
> controls any of the initialization in the subtarget dependent features
> it'll need to be part of the key to look up the subtarget. If it
doesn't
> then you'll just need to check the attribute, as you said, in the
> pass/lowering/thingy.
>
>
Yes, I saw that code. I was thinking I would have to change the map's key
type to a tuple of strings and options, but perhaps there is a more
efficient way to do the look up.

> c) Which command line options need to be part of this interface? I
don't
> necessarily think all of them should which could turn some of these into
> subtarget features that can just be added on to the functions as they go.
> If anything can't change between translation units then a module level
flag
> that errors on merge would be applicable. I'd prefer not to use module
> level flags for things that can just be put on every function. The module
> level flags could be read into a subtarget specific global target options
> flag (ala soft-float).
>
>
Is there a way to attach a subtarget feature string to a function without
using AttributeSet? I'm not sure I understand why subtarget features are
preferable to function attributes.

> d) linkonce_odr functions with different attributes
>
> e) How to organize these so that it's easy for a particular target to
know
> what attributes it might put on a function or module?
>
> Probably more :)
>
> Other possible ideas that I've discussed or thought about, but
haven't
>> implemented:
>>
>> 1. Treat cl::opt options as overrides, possibly by setting a bit that
>> indicates the option has been specified in the command line.
>>
>> 2. Extend the idea proposed in the discussion on llvm-dev about
removing
>> static initializer for command line options:
>>
>> http://lists.cs.uiuc.edu/pipermail/llvmdev/2014-August/075886.html
>>
>> I looked at the code that was checked in to trunk and it seems to me
that
>> it isn't possible to have storage for command line options on a
>> per-function storage basis yet, which I think is necessary if the
functions
>> in a module have different sets of command line options.
>>
>>
> I don't think either of these are a good idea.
>
> Thoughts?
>
> -eric
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20141114/9e26ab79/attachment.html>

> 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