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

Pete Cooper peter_cooper at apple.com
Wed Feb 18 16:22:35 PST 2015 

> On Feb 18, 2015, at 4:19 PM, Sean Silva <chisophugis at gmail.com> wrote:
> 
> 
> 
> On Fri, Feb 13, 2015 at 11:13 AM, Pete Cooper <peter_cooper at apple.com <mailto:peter_cooper at apple.com>> wrote:
> 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.
> 
> I'm wondering if we could wire the LLC options to just change what Duncan's OP called "default module options"? If we have a way to transport these settings to the backend in the module as data, having a path in the code seems redundant; better to canonicalize on always getting this information to the backend through the module. As a bonus, that helps with Reid's concern too, I think.
Sounds good to me.  It would be really nice to have a single place to look to see the default options, and you’ll see them every time you dump the module too which is a bonus.

Pete
> 
> -- Sean Silva
>  
> 
> Thanks,
> Pete
> > On Feb 12, 2015, at 4:02 PM, Jim Grosbach <grosbach at apple.com <mailto:grosbach at apple.com>> wrote:
> >
> >
> >> On Feb 12, 2015, at 4:00 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> >>
> >> +grosbach
> >>
> >>> On 2015-Feb-12, at 14:45, Reid Kleckner <rnk at google.com <mailto: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 <mailto: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 <mailto:LLVMdev at cs.uiuc.edu>         http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>
> >>>
> >>>
> >>
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>         http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev>
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu <mailto:LLVMdev at cs.uiuc.edu>         http://llvm.cs.uiuc.edu <http://llvm.cs.uiuc.edu/>
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev <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/f01c3bd3/attachment.html>

More information about the llvm-dev mailing list