llvm dev - Feb 2015 - [LLVMdev] [RFC] Storing default function attributes on the module

As we encode more CodeGen and target-specific options in bitcode to
support LTO, we risk crippling `llc` as a debugging tool.  In
particular, `llc` command-line options are generally ignored when a
function has an attribute set explicitly, but the plan of record is for
`clang` to explicitly encode all (or most) CodeGen options -- even the
target defaults.

Changing `clang` to store target defaults on the module will allow us to
continue to override them when running `llc`.  The right precedence
would be:

  1. Explicit attributes set on the function.
  2. `llc` command-line options.
  3. Default function attributes stored on the module.

(Outside of `llc`, skip step 2.)

In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),
defaults should be pushed down as explicit function attributes.

Default function-level attributes
================================
I've attached patches with a reference implementation.

  - 0001: Canonicalize access to function attributes to use
    `getFnAttribute()` and `hasFnAttribute()`.  (This seems like a nice
    cleanup regardless?)
  - 0002: Add the feature.
  - 0003: Use it in `clang` for function attributes based solely on
    `CodeGenOptions`.

They look like this in assembly:

    attributes default = { "no-frame-pointer-elim"="false" }

Limitations
==========
There are a few limitations with this approach (at least, with my
reference implementation).

  - `Function::getAttributes()` only reflects the explicitly specified
    attributes, skipping those set as module defaults.
  - If an enum attribute is set as a default, there's no way for a
    function-attribute to override it.  In practice, we could avoid the
    feature for enum attributes.
  - `CallSite` instructions store function-level attributes, but don't
    forward to the module-level defaults.  There are places (like the
    calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we
    use the callee function attributes to set the call site attributes.
    In practice, we could avoid the feature for attributes that are
    meaningful for call sites.
  - Intrinsics' attributes are independent of `CodeGenOptions`, and set
    via `Instrinsic::getAttributes()`.  With this change they'd inherit
    the default attributes like other functions.  Is this a problem?
    If so, we can add a flag on `Function` that inhibits forwarding to
    the defaults.

Thoughts?  Other ideas for solving the `llc` problem?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-IR-Canonicalize-access-to-function-attributes-N-llvm.patch
Type: application/octet-stream
Size: 63228 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/5ea77bd6/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-IR-Add-default-function-attributes-to-Module-llvm.patch
Type: application/octet-stream
Size: 31310 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/5ea77bd6/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-CodeGen-Set-module-wide-attributes-on-the-modu-clang.patch
Type: application/octet-stream
Size: 4210 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150211/5ea77bd6/attachment-0002.obj>

Are llc command line options all that critical? It's not that hard to edit
the attributes directly or remove them with sed.

The less codegen depends on llc command line flags, the better, IMO.

On Wed, Feb 11, 2015 at 11:34 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
> As we encode more CodeGen and target-specific options in bitcode to
> support LTO, we risk crippling `llc` as a debugging tool.  In
> particular, `llc` command-line options are generally ignored when a
> function has an attribute set explicitly, but the plan of record is for
> `clang` to explicitly encode all (or most) CodeGen options -- even the
> target defaults.
>
> Changing `clang` to store target defaults on the module will allow us to
> continue to override them when running `llc`.  The right precedence
> would be:
>
>   1. Explicit attributes set on the function.
>   2. `llc` command-line options.
>   3. Default function attributes stored on the module.
>
> (Outside of `llc`, skip step 2.)
>
> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),
> defaults should be pushed down as explicit function attributes.
>
> Default function-level attributes
> ================================>
> I've attached patches with a reference implementation.
>
>   - 0001: Canonicalize access to function attributes to use
>     `getFnAttribute()` and `hasFnAttribute()`.  (This seems like a nice
>     cleanup regardless?)
>   - 0002: Add the feature.
>   - 0003: Use it in `clang` for function attributes based solely on
>     `CodeGenOptions`.
>
> They look like this in assembly:
>
>     attributes default = {
"no-frame-pointer-elim"="false" }
>
> Limitations
> ==========>
> There are a few limitations with this approach (at least, with my
> reference implementation).
>
>   - `Function::getAttributes()` only reflects the explicitly specified
>     attributes, skipping those set as module defaults.
>   - If an enum attribute is set as a default, there's no way for a
>     function-attribute to override it.  In practice, we could avoid the
>     feature for enum attributes.
>   - `CallSite` instructions store function-level attributes, but don't
>     forward to the module-level defaults.  There are places (like the
>     calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we
>     use the callee function attributes to set the call site attributes.
>     In practice, we could avoid the feature for attributes that are
>     meaningful for call sites.
>   - Intrinsics' attributes are independent of `CodeGenOptions`, and set
>     via `Instrinsic::getAttributes()`.  With this change they'd inherit
>     the default attributes like other functions.  Is this a problem?
>     If so, we can add a flag on `Function` that inhibits forwarding to
>     the defaults.
>
> Thoughts?  Other ideas for solving the `llc` problem?
>
>
> _______________________________________________
> 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/20150212/5e2f2539/attachment.html>

+grosbach
> On 2015-Feb-12, at 14:45, Reid Kleckner <rnk at google.com> wrote:
> 
> Are llc command line options all that critical? It's not that hard to
edit the attributes directly or remove them with sed.
Maybe Jim can speak to this one better than I can, but the workflow
I've heard concerns about is:

  - Got a codegen bug (PR or whatever).
  - Want to fiddle with codegen options in `llc`, to see which ones
    affect the bug and which don't.
  - Don't want command-line options to influence attributes that
    were specified explicitly.
  - Obviously want to influence the others.

Sure, `sed` could do this, but it's manual and fairly error-prone,
and would have a pretty tough time figuring out which attributes
are there because they're target defaults vs. specified in the
source.
> The less codegen depends on llc command line flags, the better, IMO.
This doesn't make sense to me.  The only command-line flags in `llc`
are codegen options... so we remove all `llc` flags?

I'm not suggesting we push more command-line flags through CodeGen;
I just don't want `llc` to *break*.  (IMO, `llc` could/should just
modify the module-level defaults I've added here, but that's not
part of this proposal since there seem to be a ton of weird issues
with command-line options and I don't really want to get involved.
Just looking to maintain current functionality.)
> On Wed, Feb 11, 2015 at 11:34 PM, Duncan P. N. Exon Smith <dexonsmith at
apple.com> wrote:
> As we encode more CodeGen and target-specific options in bitcode to
> support LTO, we risk crippling `llc` as a debugging tool.  In
> particular, `llc` command-line options are generally ignored when a
> function has an attribute set explicitly, but the plan of record is for
> `clang` to explicitly encode all (or most) CodeGen options -- even the
> target defaults.
> 
> Changing `clang` to store target defaults on the module will allow us to
> continue to override them when running `llc`.  The right precedence
> would be:
> 
>   1. Explicit attributes set on the function.
>   2. `llc` command-line options.
>   3. Default function attributes stored on the module.
> 
> (Outside of `llc`, skip step 2.)
> 
> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),
> defaults should be pushed down as explicit function attributes.
> 
> Default function-level attributes
> ================================> 
> I've attached patches with a reference implementation.
> 
>   - 0001: Canonicalize access to function attributes to use
>     `getFnAttribute()` and `hasFnAttribute()`.  (This seems like a nice
>     cleanup regardless?)
>   - 0002: Add the feature.
>   - 0003: Use it in `clang` for function attributes based solely on
>     `CodeGenOptions`.
> 
> They look like this in assembly:
> 
>     attributes default = {
"no-frame-pointer-elim"="false" }
> 
> Limitations
> ==========> 
> There are a few limitations with this approach (at least, with my
> reference implementation).
> 
>   - `Function::getAttributes()` only reflects the explicitly specified
>     attributes, skipping those set as module defaults.
>   - If an enum attribute is set as a default, there's no way for a
>     function-attribute to override it.  In practice, we could avoid the
>     feature for enum attributes.
>   - `CallSite` instructions store function-level attributes, but don't
>     forward to the module-level defaults.  There are places (like the
>     calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we
>     use the callee function attributes to set the call site attributes.
>     In practice, we could avoid the feature for attributes that are
>     meaningful for call sites.
>   - Intrinsics' attributes are independent of `CodeGenOptions`, and set
>     via `Instrinsic::getAttributes()`.  With this change they'd inherit
>     the default attributes like other functions.  Is this a problem?
>     If so, we can add a flag on `Function` that inhibits forwarding to
>     the defaults.
> 
> Thoughts?  Other ideas for solving the `llc` problem?
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> 
>

On Wed, Feb 11, 2015 at 11:34 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
> As we encode more CodeGen and target-specific options in bitcode to
> support LTO, we risk crippling `llc` as a debugging tool.  In
> particular, `llc` command-line options are generally ignored when a
> function has an attribute set explicitly, but the plan of record is for
> `clang` to explicitly encode all (or most) CodeGen options -- even the
> target defaults.
>
> Changing `clang` to store target defaults on the module will allow us to
> continue to override them when running `llc`.  The right precedence
> would be:
>
>   1. Explicit attributes set on the function.
>   2. `llc` command-line options.
>   3. Default function attributes stored on the module.
>
> (Outside of `llc`, skip step 2.)
>
> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),
> defaults should be pushed down as explicit function attributes.
>
> Default function-level attributes
> ================================>
> I've attached patches with a reference implementation.
>
>   - 0001: Canonicalize access to function attributes to use
>     `getFnAttribute()` and `hasFnAttribute()`.  (This seems like a nice
>     cleanup regardless?)
>   - 0002: Add the feature.
>
Random comment, but this patch changes the IR without any updates to
LangRef (or its sub-pages).

-- Sean Silva

>   - 0003: Use it in `clang` for function attributes based solely on
>     `CodeGenOptions`.
>
> They look like this in assembly:
>
>     attributes default = {
"no-frame-pointer-elim"="false" }
>
> Limitations
> ==========>
> There are a few limitations with this approach (at least, with my
> reference implementation).
>
>   - `Function::getAttributes()` only reflects the explicitly specified
>     attributes, skipping those set as module defaults.
>   - If an enum attribute is set as a default, there's no way for a
>     function-attribute to override it.  In practice, we could avoid the
>     feature for enum attributes.
>   - `CallSite` instructions store function-level attributes, but don't
>     forward to the module-level defaults.  There are places (like the
>     calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we
>     use the callee function attributes to set the call site attributes.
>     In practice, we could avoid the feature for attributes that are
>     meaningful for call sites.
>   - Intrinsics' attributes are independent of `CodeGenOptions`, and set
>     via `Instrinsic::getAttributes()`.  With this change they'd inherit
>     the default attributes like other functions.  Is this a problem?
>     If so, we can add a flag on `Function` that inhibits forwarding to
>     the defaults.
>
> Thoughts?  Other ideas for solving the `llc` problem?
>
>
> _______________________________________________
> 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/20150218/a9b029d6/attachment.html>