[PATCH] D11815: Pass subtarget feature "force-align-stack"

[Eric Christopher via cfe-commits]

On Thu, Aug 13, 2015 at 5:16 PM hfinkel at anl.gov <hfinkel at anl.gov> wrote:

> hfinkel added a comment.
>
> In http://reviews.llvm.org/D11815#224169, @echristo wrote:
>
> > 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.
>
>
> 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.
>
>
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.

I'm guessing from the comment here that you're talking about something on
the order of:

"force-stack-align"="true"

versus something like:

target-features="+force-stack-align".

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?

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.

-eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150814/6a9a7032/attachment.html>