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

Wed Feb 18 16:19:48 PST 2015

On Fri, Feb 13, 2015 at 11:13 AM, Pete Cooper <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.

-- Sean Silva

>
> 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
>
>
> _______________________________________________
> 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/573b72f3/attachment.html>