<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 17, 2015 at 12:41 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><br><div class="gmail_quote"><div><div class="h5"><div dir="ltr">On Mon, Aug 17, 2015 at 11:57 AM Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" target="_blank">ahatanak@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Aug 13, 2015 at 5:56 PM, Eric Christopher <span dir="ltr"><<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
> Apologies, I'm really resistant to more things being used in<br>
> TargetOptions and I was (perhaps mistakenly) under the impression<br>
> that you wanted to move it to TargetOptions without an IR<br>
> serialization. We need all options to have that sort of<br>
> serialization right? :)<br>
<br>
Absolutely, they all need function-level serialization for LTO to work. We're definitely both on the same page there :)<br>
<br></blockquote><div><br></div></span><div>Cool.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> In this case it's for the -mstackrealign<br>
> option and we need to keep that if it's going to work for separate<br>
> compilation.<br>
><br>
><br>
> I'm guessing from the comment here that you're talking about<br>
> something on the order of:<br>
><br>
><br>
> "force-stack-align"="true"<br>
><br>
><br>
><br>
> versus something like:<br>
><br>
><br>
> target-features="+force-stack-align".<br>
><br>
<br>
Yes.<br>
<br></blockquote><div><br></div></span><div>Yep.</div><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
><br>
> Which I can somewhat agree with if we really want to. I don't know if<br>
> this is better suited toward an actual IR level attribute though?<br>
><br>
> I moved soft-float over to a subtarget feature because it was<br>
> something used to conditionalize initialization for each subtarget.<br>
> RESET_OPTIONS needs to die a horrible death though so I don't think<br>
> we should move this to TargetOptions. If we're going to do something<br>
> then let's just add a target attribute and use that as a lookup. If<br>
> you don't want to use it as a subtarget feature (it's not clear at<br>
> all that it should be I agree), then we should just have it as a<br>
> serializable attribute.<br>
<br>
To be clear, I don't care whether it is a subtarget feature or not. But if it is a subtarget feature, we need a way of doing that in some kind of base class (either in C++ or in TableGen) so that we don't just need to copy-and-paste it into every backend. Adding a particular subtarget feature with a specific name to every target goes beyond justifiable boilerplate.<br>
<br></blockquote><div><br></div></span><div>Agreed. It's one reason the patch had sat for a while (thanks for looking btw, it spurred me to a bit of action). I had some patches that added generic subtarget features to Target.td for soft float originally and was convinced to do the per-target bit. I agree that per-target is insanely boilerplate here and we should come up with something else.</div><span><div> </div></span></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>If we aren't going to have generic subtarget features, I think we should just use function attributes for target independent code-gen options like force-align-stack.</div></div></div></div></blockquote><div><br></div></div></div><div>I've been using subtarget features for anything necessary to initialize the subtarget. Notice soft float for example.</div><span class=""><div> </div></span></div></div></blockquote><div><br></div><div>OK, a subtarget feature should be used if there is a variable of subtarget that needs initialization. force-align-stack doesn't seem to need a variable in subtarget, so it can be a function attribute.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
And, whatever we do, we really need to be consistent about it. Let's decide on a way forward and unify everything in that direction. We also have direct calls to check attributes in various places (such as 'if (MF.getFunction()->hasFnAttribute("no-frame-pointer-elim-non-leaf"))' in lib/CodeGen/TargetOptionsImpl.cpp) and we could simply add utility functions to MachineFunction if we'd like too.<br>
<br></blockquote><div><br></div></span><div>I'm all about something new here. I've got "use-soft-float"="true" autoupgrading to the particular subtarget feature now (IIRC), but these kinds of string pair features are a bit odd after a while. Perhaps either a generic target-options="stuff" on the function that gets parsed once at Function creation time? That seems nice and extensible?</div><span><font color="#888888"><div><br></div></font></span></div></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>So, this is about changing the implementation of Attribute or AttributeSet and convert "attrkind"="attrval" in the IR to something different internally? Is this supposed to fix some flaws of Attribtue or AttributeSet?</div></div></div></div></blockquote><div><br></div></span><div>No, thinking about representing certain sets of features as "attributes on functions" that are perhaps a bit more free form, but not as ... verbose as the current implementation.</div><span class="HOEnZb"><font color="#888888"><div><br></div></font></span></div></div></blockquote><div><br></div><div>Could you give some examples?</div><div><br></div><div>So for a function attribute like "force-align-stack" (can be either an enum or a string, but note that there is no "false" or "true" value as there is no need to make it a tri-state), what would the new less verbose representation look like and how would it improve the way function attributes are handled?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span class="HOEnZb"><font color="#888888"><div></div><div>-eric</div></font></span><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote"><span><font color="#888888"><div></div><div>-eric</div></font></span><span><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
 -Hal<br>
<br>
><br>
><br>
> -eric<br>
<br>
--<br>
Hal Finkel<br>
Assistant Computational Scientist<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</blockquote></span></div></div>
</blockquote></div></div></div></blockquote></span></div></div>
</blockquote></div><br></div></div>