<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 18, 2015, at 4:19 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com" class="">chisophugis@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><br class=""><div class="gmail_extra"><br class=""><div class="gmail_quote">On Fri, Feb 13, 2015 at 11:13 AM, Pete Cooper <span dir="ltr" class=""><<a href="mailto:peter_cooper@apple.com" target="_blank" class="">peter_cooper@apple.com</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Duncan<br class="">
<br class="">
The first patch is general goodness and I think should be committed now.<br class="">
<br class="">
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.<br class=""></blockquote><div class=""><br class=""></div><div class="">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.</div></div></div></div></div></blockquote>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.</div><div><br class=""></div><div>Pete</div><div><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class="gmail_extra"><div class="gmail_quote"><div class=""><br class=""></div><div class="">-- Sean Silva</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="">
Thanks,<br class="">
Pete<br class="">
<div class="HOEnZb"><div class="h5">> On Feb 12, 2015, at 4:02 PM, Jim Grosbach <<a href="mailto:grosbach@apple.com" class="">grosbach@apple.com</a>> wrote:<br class="">
><br class="">
><br class="">
>> On Feb 12, 2015, at 4:00 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class="">
>><br class="">
>> +grosbach<br class="">
>><br class="">
>>> On 2015-Feb-12, at 14:45, Reid Kleckner <<a href="mailto:rnk@google.com" class="">rnk@google.com</a>> wrote:<br class="">
>>><br class="">
>>> Are llc command line options all that critical? It's not that hard to edit the attributes directly or remove them with sed.<br class="">
>><br class="">
>> Maybe Jim can speak to this one better than I can, but the workflow<br class="">
>> I've heard concerns about is:<br class="">
>><br class="">
>> - Got a codegen bug (PR or whatever).<br class="">
>> - Want to fiddle with codegen options in `llc`, to see which ones<br class="">
>> affect the bug and which don't.<br class="">
>> - Don't want command-line options to influence attributes that<br class="">
>> were specified explicitly.<br class="">
>> - Obviously want to influence the others.<br class="">
>><br class="">
>> Sure, `sed` could do this, but it's manual and fairly error-prone,<br class="">
>> and would have a pretty tough time figuring out which attributes<br class="">
>> are there because they're target defaults vs. specified in the<br class="">
>> source.<br class="">
><br class="">
> 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.<br class="">
><br class="">
>><br class="">
>>> The less codegen depends on llc command line flags, the better, IMO.<br class="">
>><br class="">
>> This doesn't make sense to me. The only command-line flags in `llc`<br class="">
>> are codegen options... so we remove all `llc` flags?<br class="">
>><br class="">
>> I'm not suggesting we push more command-line flags through CodeGen;<br class="">
>> I just don't want `llc` to *break*. (IMO, `llc` could/should just<br class="">
>> modify the module-level defaults I've added here, but that's not<br class="">
>> part of this proposal since there seem to be a ton of weird issues<br class="">
>> with command-line options and I don't really want to get involved.<br class="">
>> Just looking to maintain current functionality.)<br class="">
>><br class="">
>>> On Wed, Feb 11, 2015 at 11:34 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com" class="">dexonsmith@apple.com</a>> wrote:<br class="">
>>> As we encode more CodeGen and target-specific options in bitcode to<br class="">
>>> support LTO, we risk crippling `llc` as a debugging tool. In<br class="">
>>> particular, `llc` command-line options are generally ignored when a<br class="">
>>> function has an attribute set explicitly, but the plan of record is for<br class="">
>>> `clang` to explicitly encode all (or most) CodeGen options -- even the<br class="">
>>> target defaults.<br class="">
>>><br class="">
>>> Changing `clang` to store target defaults on the module will allow us to<br class="">
>>> continue to override them when running `llc`. The right precedence<br class="">
>>> would be:<br class="">
>>><br class="">
>>> 1. Explicit attributes set on the function.<br class="">
>>> 2. `llc` command-line options.<br class="">
>>> 3. Default function attributes stored on the module.<br class="">
>>><br class="">
>>> (Outside of `llc`, skip step 2.)<br class="">
>>><br class="">
>>> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),<br class="">
>>> defaults should be pushed down as explicit function attributes.<br class="">
>>><br class="">
>>> Default function-level attributes<br class="">
>>> =================================<br class="">
>>><br class="">
>>> I've attached patches with a reference implementation.<br class="">
>>><br class="">
>>> - 0001: Canonicalize access to function attributes to use<br class="">
>>> `getFnAttribute()` and `hasFnAttribute()`. (This seems like a nice<br class="">
>>> cleanup regardless?)<br class="">
>>> - 0002: Add the feature.<br class="">
>>> - 0003: Use it in `clang` for function attributes based solely on<br class="">
>>> `CodeGenOptions`.<br class="">
>>><br class="">
>>> They look like this in assembly:<br class="">
>>><br class="">
>>> attributes default = { "no-frame-pointer-elim"="false" }<br class="">
>>><br class="">
>>> Limitations<br class="">
>>> ===========<br class="">
>>><br class="">
>>> There are a few limitations with this approach (at least, with my<br class="">
>>> reference implementation).<br class="">
>>><br class="">
>>> - `Function::getAttributes()` only reflects the explicitly specified<br class="">
>>> attributes, skipping those set as module defaults.<br class="">
>>> - If an enum attribute is set as a default, there's no way for a<br class="">
>>> function-attribute to override it. In practice, we could avoid the<br class="">
>>> feature for enum attributes.<br class="">
>>> - `CallSite` instructions store function-level attributes, but don't<br class="">
>>> forward to the module-level defaults. There are places (like the<br class="">
>>> calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we<br class="">
>>> use the callee function attributes to set the call site attributes.<br class="">
>>> In practice, we could avoid the feature for attributes that are<br class="">
>>> meaningful for call sites.<br class="">
>>> - Intrinsics' attributes are independent of `CodeGenOptions`, and set<br class="">
>>> via `Instrinsic::getAttributes()`. With this change they'd inherit<br class="">
>>> the default attributes like other functions. Is this a problem?<br class="">
>>> If so, we can add a flag on `Function` that inhibits forwarding to<br class="">
>>> the defaults.<br class="">
>>><br class="">
>>> Thoughts? Other ideas for solving the `llc` problem?<br class="">
>>><br class="">
>>><br class="">
>>> _______________________________________________<br class="">
>>> LLVM Developers mailing list<br class="">
>>> <a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
>>><br class="">
>>><br class="">
>><br class="">
><br class="">
><br class="">
> _______________________________________________<br class="">
> LLVM Developers mailing list<br class="">
> <a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
LLVM Developers mailing list<br class="">
<a href="mailto:LLVMdev@cs.uiuc.edu" class="">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu/" target="_blank" class="">http://llvm.cs.uiuc.edu</a><br class="">
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank" class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br class="">
</div></div></blockquote></div><br class=""></div></div>
</div></blockquote></div><br class=""></body></html>