On Mon, Aug 17, 2015 at 11:57 AM Akira Hatanaka <ahata...@gmail.com> wrote:
> On Thu, Aug 13, 2015 at 5:56 PM, Eric Christopher <echri...@gmail.com> > wrote: > >> >>> > 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. >> >> > > 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. > I've been using subtarget features for anything necessary to initialize the subtarget. Notice soft float for example. > > >> 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? >> >> > 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? > 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. -eric > > >> -eric >> >> >>> -Hal >>> >>> > >>> > >>> > -eric >>> >>> -- >>> Hal Finkel >>> Assistant Computational Scientist >>> Leadership Computing Facility >>> Argonne National Laboratory >>> >>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits