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

[Jim Grosbach]

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