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

[Eric Christopher via cfe-commits]

>
>
> > 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? :)
>
> Absolutely, they all need function-level serialization for LTO to work.
> We're definitely both on the same page there :)
>
>
Cool.


> > 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".
> >
>
> Yes.
>
>
Yep.


> >
> > 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.
>
> 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.
>
>
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.


> 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.
>
>
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?

-eric


>  -Hal
>
> >
> >
> > -eric
>
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150814/6f7808ea/attachment.html>