On Fri, 2014-09-05 at 10:43 -0600, Jeff Law wrote: > On 09/05/14 09:32, tsaund...@mozilla.com wrote: > > From: Trevor Saunders <tsaund...@mozilla.com> > > > > Hi, > > > > Doing this means we get to remove a fair chunk of runtime checking. > > > > bootstrapped + regtested on x86_64-unknown-linux-gnu with no regressions. > > config-list.mk with this and the next patch is ongoing. ok? > > > > Trev > > > > gcc/ > > > > * config/i386/i386.c, config/i386/i386.md, config/mep/mep.c, > > config/mips/mips.c, config/nios2/nios2.c, config/s390/s390.c, > > config/sh/sh.c, reorg.c: Use rtx_code_label::nuses and > > rtx_code_label::set_nuses where possible. > > * rtl.h (rtx_code_label::nuses): New member function. > > (rtx_code_label::set_nuses): Likewise. > Is there some reason why we don't fix every reference to LABEL_NUSES to > use the new member functions? My concern here is that we've now got two > ways actively used to get at that information, the old LABEL_NUSES and > the new member functions. Obviously we really just want one blessed way > to get/set that information. If this is a short term situation, then > I'd be inclined to say OK, but I don't see further patches which sort > out the ~200 or so LABEL_NUSES users. > > THe approach David has taken for similar situations has been to first > introduce the get/set methods, convert everything to set the get/set > method, dealing with the type fallout that occurrs around that, then > collapsing back to the old macro. Is there some reason we can not or > should not do that here?
If it's a macro that's simply a direct field access into a struct (e.g. BB_HEADER), then yes, but for those that are wrappers around e.g. XEXP I've been keeping them as inline functions, so that the type system detects incorrect node types as compile-time errors. One other aspect of my approach is that (believe it or not) I'm trying to minimize the size of the changes, to avoid introducing pain when backporting bugfixes from trunk to the branches. My goal here is type-safety, with readability as a secondary benefit. I think it's a good idea for us to add methods that let us replace e.g. XEXP (x, 0) accessors with descriptive names, and have been doing so, and I prefer doing this as methods for new code. However, when the accessor already has a descriptive name, like LABEL_NUSES, I think it's enough to convert them to inline functions and tighten up the params and return type to express things. I'm not sure the cost/benefit of *additionally* converting them to be methods is worth it, given that it means changing the spelling at every callsite. Dave
