[LLVMdev] [RFC] Storing default function attributes on the module

[Duncan P. N. Exon Smith]

> On 2015-Feb-13, at 11:13, Pete Cooper <peter_cooper at apple.com> wrote:
> 
> Hi Duncan
> 
> The first patch is general goodness and I think should be committed now.

That's what I thought.

I'll start committing.

> 
> The other 2 LGTM.  Unless anyone fundamentally objects to module attributes, or has feedback on the patches themselves, then please commit.  I didn’t see any problems with them.

Cool.  I figure I'll sit on them at least the next weekly mail goes
out, just in case someone finds a problem with the direction.

> Thanks,
> Pete
>> On Feb 12, 2015, at 4:02 PM, Jim Grosbach <grosbach at apple.com> wrote:
>> 
>> 
>>> On Feb 12, 2015, at 4:00 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
>>> 
>>> +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.
>> 
>> Yep. Duncan summarized it nicely. Breaking llc’s ability to use these options to debug problems will be a *very* big usability loss for LLVM backend devs.
>> 
>>> 
>>>> 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
>>>> 
>>>> 
>>> 
>> 
>> 
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>