On Mon, Aug 17, 2015 at 12:41 PM, Eric Christopher <echri...@gmail.com> wrote:
> > > 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. > > 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. > >> >>> 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. > > Could you give some examples? 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? > -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