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

[Pete Cooper]

Hi Duncan

The first patch is general goodness and I think should be committed now.

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.

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