[RFC] Embedding command line options in bitcode (PR21471)

[ahatanak]

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.

llvm-pr21471.patch (10.1 KB)

clang-pr21471.patch (844 Bytes)

[echristo]

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:

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

[dexonsmith]

+chrisb

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.

21471 – Encoded compilation options in bitcode file

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?

[dexonsmith]

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:

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.

21471 – Encoded compilation options in bitcode file

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

[Chris_Bieneman1]

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.

+chrisb

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

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

[echristo]

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.

shrug Just hack it if necessary.

It should be as simple as checking the function attribute for each pass to decide when to run,

Right, these are easy.

Yep.

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?

Agreed, I don’t know of many of these anyhow outside of the atomic lowering.

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?

-mips16 is the trivial example that already exists. -mabi= would probably be another when you can change it per function. (attribute((pcs(“aapcs”)))). Probably more, just those are the ones that come to mind.

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

Right. Which one wins? You’ll get inconsistent results depending on ordering. Of course, you’d get that without linking them, but we should do some sort of checking ideally.

Not necessarily objecting to Akira’s plan at the moment in case that was a concern.

-eric

[dexonsmith]

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.

+chrisb

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.

21471 – Encoded compilation options in bitcode file

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.

To be clear, that's what this is supposed to be solving.

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: ⚙ D6207 We need an OptionStore abstraction for storing debug option data.

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,

ParseCommandLineOptions gets called directly from clang, twice.
That's the mechanism for passing in options via `-mllvm` and
`-backend-option`.

Clang could be changed to call the OptionStore/OptionRegistry once it
exists though.

we could make encoding and decoding the stored options associated with a module part of loading and storing the module.

Right. These attributes get stored in the IR, either as function
attributes or in the module-level metadata, depending on whether it's
per-function or per-translation-unit. The problem is, how do you
pass them in from `clang` in the first place, and once the bitcode is
saved, how do the encoded functions interact with options passed from
`llc`?

[dexonsmith]

You mean some lib/Linker tests, to make sure that the version of the
function that wins has its own attributes attached, instead of those
of one of the losing functions? Yup, makes sense.

[echristo]

Or perhaps a warning on “you said these two things were identical… they’re not” with some calls into the backend.

-eric

[pete]

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.

+chrisb

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.

Ideally this will be a small set as it could get expensive to represent otherwise. I’ll get to why later.

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.

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.

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:

• Always choose 150 for this function, because thats what it was tagged with
• Always choose the default, so if we change ToT to default 200, you choose 200.

Now if you have to store all ‘interesting’ options, the set of things you store could start to get quite large quite quickly.

2. How to handle cases where two functions in a module have different sets of command line options?

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.

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

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.

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

class LoopUnrollPass {
unsigned Threshold = …
doIniit… {
if (cl opt has value)
Threshold = ‘cl opt value’
if (function.getattribute(‘unroll-threshold’)
Threadhold = function.getAttributeAsInteger(‘unroll-threshold’)
}

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.

Thanks,
Pete

[echristo]

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

[ahatanak]

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:

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.

21471 – Encoded compilation options in bitcode file

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.

[ahatanak]

functions in a module require different optimization options. But for now,
I'm going to assume that we can handle most cases by just checking the
function attribute to decide whether or not to run a pass.

[Bob_Wilson]

+chrisb

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.

21471 – Encoded compilation options in bitcode file

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

[Chris_Bieneman1]

+chrisb

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

[ahatanak]

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.

+chrisb

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.

21471 – Encoded compilation options in bitcode file

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.

Ideally this will be a small set as it could get expensive to represent
otherwise. I’ll get to why later.

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.

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.

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.

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:
- Always choose 150 for this function, because thats what it was tagged
with
- Always choose the default, so if we change ToT to default 200, you
choose 200.

Now if you have to store all ‘interesting’ options, the set of things you
store could start to get quite large quite quickly.

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.

[pete]

Thanks. Thats a really useful document!

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.

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.

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.

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.

Thanks,
Pete

[echristo]

+chrisb

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

[ahatanak]

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>. This is just a temporary change to ease debugging and should be reverted to a StringMap or changed to another type of map later.

llvm-pr21471.patch (20.1 KB)

clang-pr21471.patch (2.86 KB)

[echristo]

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