<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Feb 24, 2015 at 2:53 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class=""><br>
> On 2015-Feb-24, at 13:25, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:<br>
><br>
> Hi Duncan,<br>
><br>
> Been thinking about this a bit and a few comments/questions. I may have misunderstood some things in your mail though so please feel free to explain at me :)<br>
><br>
><br>
> Changing `clang` to store target defaults on the module will allow us to<br>
> continue to override them when running `llc`. The right precedence<br>
> would be:<br>
><br>
> 1. Explicit attributes set on the function.<br>
> 2. `llc` command-line options.<br>
> 3. Default function attributes stored on the module.<br>
><br>
> (Outside of `llc`, skip step 2.)<br>
><br>
> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),<br>
> defaults should be pushed down as explicit function attributes.<br>
><br>
> I think there are a few options/backend communications that aren't/haven't gone this way yet that are probably worth considering:<br>
><br>
> MCTargetOptions/TargetOptions: Basically a grab bag of functionality, some of which is duplicated via attributes and some that's not. I think at least some of these should be replaced by module flags, e.g. ABI.<br>
><br>
> Random backend flags: Anything for debugging.<br>
><br>
> I'm thinking things that are either set as Init(true/false) and affect things at a global level and not just the function level.<br>
<br>
</span>(I think my lib/Linker comment was unclear. See below.)<br>
<br>
Not at all trying to say that *everything* should be a function<br>
attribute; global-level stuff should absolutely be module flags.<br>
<br>
I'm just talking about infrastructure for the things that *are*<br>
function-level.<br>
<span class=""><br>
><br>
> They look like this in assembly:<br>
><br>
> attributes default = { "no-frame-pointer-elim"="false" }<br>
><br>
><br>
> So, how do you see module merging and defaults working? (Yes, there were testcases, but let's go with prose here. I found the testcases a bit hard to reason.)<br>
<br>
</span>This is where my lib/Linker comment applies:<br>
<span class=""><br>
>> In `lib/Linker` (i.e., `llvm-lto`, `llvm-link`, `libLTO.dylib`),<br>
>> defaults should be pushed down as explicit function attributes.<br>
<br>
</span>^ This is how I see module merging and defaults working: push the<br>
defaults down to explicit function attributes. So there wouldn't<br>
be any default function attributes in the output of `llvm-link`.<br>
This means that `llc` will still have trouble overriding attributes<br>
in the output of merged modules -- but at least it can handle the<br>
output of `clang` without trouble. In the future we could try to<br>
be more intelligent about merged modules, and keep common options<br>
in the default set.<br>
<span class=""><br>
><br>
> Limitations<br>
> ===========<br>
><br>
> There are a few limitations with this approach (at least, with my<br>
> reference implementation).<br>
><br>
> - `Function::getAttributes()` only reflects the explicitly specified<br>
> attributes, skipping those set as module defaults.<br>
><br>
> Ick. Pretty severe limitation? I.e. how would it work to test general attributes on a function during code gen?<br>
<br>
</span>As long as everyone calls `Function::hasFnAttribute()`, there's no<br>
problem. This proposal basically turns it into a bug to access<br>
them directly; you need to go through `Function::hasFnAttribute()`<br>
to get the right answer. (Not sure if there's a better way?)<br>
<span class=""><br>
><br>
> - If an enum attribute is set as a default, there's no way for a<br>
> function-attribute to override it. In practice, we could avoid the<br>
> feature for enum attributes.<br>
><br>
> Hrm. This seems like a pretty severe limitation? Anything come to mind in practice.<br>
<br>
</span>In the `Attribute` format, mutually exclusive attributes aren't<br>
related at all (they're not inherently mutually exclusive). To<br>
make them overridable, we'd need a completely new design for<br>
enum attributes.<br>
<br>
As a result, this proposal only improves `llc` for string-based<br>
attributes. I don't see that as a problem... the string-based<br>
attributes are more flexible anyway. Maybe `Module` should only<br>
allow `setDefaultFnAttribute()` for string attributes though?<br>
<br>
(Some more context on why enum attributes can't really be<br>
overridden. This isn't just a problem for enum attributes that<br>
are mutually exclusive. Consider:<br>
<br>
attributes defaults = { noreturn }<br>
<br>
Besides being somewhat insane, there's no `return` attribute,<br>
so you can't really override it. I suppose one idea would be to<br>
explicitly mark a function `~noreturn` or something:<br>
<br>
define void @foo() ~noreturn { ; Ignore module default noreturn.<br>
<br>
Not sure if this direction is a good one though.)<br>
<span class=""><br>
><br>
> - `CallSite` instructions store function-level attributes, but don't<br>
> forward to the module-level defaults. There are places (like the<br>
> calls to `EmitUnaryFloatFnCall()` in `-simplify-libcalls`) where we<br>
> use the callee function attributes to set the call site attributes.<br>
> In practice, we could avoid the feature for attributes that are<br>
> meaningful for call sites.<br>
><br>
> Sure.<br>
><br>
> - Intrinsics' attributes are independent of `CodeGenOptions`, and set<br>
> via `Instrinsic::getAttributes()`. With this change they'd inherit<br>
> the default attributes like other functions. Is this a problem?<br>
> If so, we can add a flag on `Function` that inhibits forwarding to<br>
> the defaults.<br>
><br>
><br>
> Seems reasonable.<br>
><br>
> Thoughts? Other ideas for solving the `llc` problem?<br>
><br>
><br>
> I think this is a good start, I think I'd like to worry about some of the other issues in advance before we start incrementally changing things though. (Also, I really have no other ideas :)<br>
><br>
<br>
<br>
</span>So Akira had an idea at the end of last week that I don't<br>
think made it onto the list, and it's worth considering as an<br>
alternative:<br>
<br>
Add a bit to attributes indicating whether or not they're<br>
overridable (i.e., they're overridable if they're target<br>
defaults; they're not overridable if they've been explicitly<br>
specified somehow).<br>
<br>
Here's some straw-man syntax:<br>
<br>
attributes #0 = { noreturn ssp? "att1"="1" "att2"="2"? }<br>
<br>
Where:<br>
<br>
- `noreturn` and `"att1"="1"` are required.<br>
- `ssp` and `"att2"="2"` can be overridden (e.g., by `llc`).<br>
<br>
(Alternately, but equivalently:<br>
<br>
attributes #0 = { noreturn! ssp "att1"="1"! "att2"="2" }<br>
<br>
I like this syntax better, but it would cause more churn, and<br>
`!` is so far reserved for metadata.)<br>
<br>
Whatever the syntax, the idea is: `llc` resets/deletes<br>
attributes on every function to match what's specified on the<br>
command-line. In the above example, functions with attribute<br>
set #0 could have `ssp` and `"att2"` overridden via the `llc`<br>
command-line, but not `noreturn` and `"att1"`.<br>
<br>
To compare it to my proposal:<br>
<br>
- Storing a default attribute set (my proposal) makes it<br>
easier to look at and set the defaults. Applying `llc`<br>
command-line options is easy, too -- just override the<br>
default attribute set on the module -- although it doesn't<br>
really work on the output of `lib/Linker`.<br>
- Storing a bit on each attribute (Akira's proposal) handles<br>
more cases. Nothing needs to be done in `lib/Linker`<br>
(`llc` is able to override the output of `llvm-link`),<br>
and it doesn't have a disconnect between `hasFnAttribute()`<br>
and `getAttributes().hasAttribute(FunctionIndex)`.<br>
<br>
Awkwardly, having written that out, I'm kind of leaning toward<br>
it myself right now (maybe I'm fickle?) -- it seems to have<br>
fewer limitations. The main thing I prefer about my proposal<br>
is that it's easier to change the default attributes when<br>
modifying an assembly file by hand, but I suppose we could<br>
write a tool for that?<br></blockquote><div><br></div><div>I think the tool approach deserves a bit of attention for the original usecase of trying different target attributes to see if they tickle a bug. Would it be feasible to have a purpose-built tool `llvm-attr-mutate` (bikeshed) so you can do `cat foo.bc | llvm-attr-mutate .... | llc`. The arguments to llvm-attr-mutate could then be made as convenient/powerful as needed for this debugging task.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5">_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
</div></div></blockquote></div><br></div></div>