<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 13, 2015 at 5:16 PM <a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a> <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">hfinkel added a comment.<br>
<br>
In <a href="http://reviews.llvm.org/D11815#224169" rel="noreferrer" target="_blank">http://reviews.llvm.org/D11815#224169</a>, @echristo wrote:<br>
<br>
> No, RESET_OPTION isn't the right way to do this either. For exactly this sort of reason. You can't actually represent all of the code and options this way in the IR. If you can't do that then it's a non-starter.<br>
<br>
<br>
Can you please be more specific, what is it that we cannot represent? TargetOptions represents the target-generic code-generation options for the current function (where the "current function" bit works because the options get reset based on the current function attributes). That's the design we have now. And sticking with that pattern, even if we're going to change the overall scheme later, is better than the code duplication and inconsistency proposed here.<br><br></blockquote><div><br></div><div>Apologies, I'm really resistant to more things being used in TargetOptions and I was (perhaps mistakenly) under the impression that you wanted to move it to TargetOptions without an IR serialization. We need all options to have that sort of serialization right? :) In this case it's for the -mstackrealign option and we need to keep that if it's going to work for separate compilation.</div><div><br></div><div>I'm guessing from the comment here that you're talking about something on the order of:</div><div><br></div><div>"force-stack-align"="true"<br></div><div><br></div><div>versus something like:</div><div><br></div><div>target-features="+force-stack-align".</div><div><br></div><div>Which I can somewhat agree with if we really want to. I don't know if this is better suited toward an actual IR level attribute though?</div><div><br></div><div>I moved soft-float over to a subtarget feature because it was something used to conditionalize initialization for each subtarget. RESET_OPTIONS needs to die a horrible death though so I don't think we should move this to TargetOptions. If we're going to do something then let's just add a target attribute and use that as a lookup. If you don't want to use it as a subtarget feature (it's not clear at all that it should be I agree), then we should just have it as a serializable attribute.</div><div><br></div><div>-eric</div></div></div>