This is the mail archive of the
mailing list for the GCC project.
Re: PING: [PATCH] Add SUBREG_REG_P predicate and change REG_SUBREG_P to be global
- From: Richard Guenther <rguenther at suse dot de>
- To: Olivier Hainque <hainque at adacore dot com>
- Cc: Shujing Zhao <pearly dot zhao at oracle dot com>, gcc-patches at gcc dot gnu dot org, Paolo Bonzini <paolo dot bonzini at gmail dot com>, Paolo Carlini <paolo dot carlini at oracle dot com>
- Date: Tue, 23 Jun 2009 11:17:46 +0200 (CEST)
- Subject: Re: PING: [PATCH] Add SUBREG_REG_P predicate and change REG_SUBREG_P to be global
- References: <4A407B3D.firstname.lastname@example.org> <20090623082932.GA9870@cardhu.act-europe.fr>
On Tue, 23 Jun 2009, Olivier Hainque wrote:
> Eventhough I'm not maintainer of the involved area, I'd like to
> voice an opinion here:
> +/* Predicate yielding nonzero iff X is a subreg of a register. */
> +#define SUBREG_REG_P(X) \
> + (GET_CODE (X) == SUBREG && REG_P (SUBREG_REG (X)))
> +/* Predicate yielding nonzero iff X is a register or subreg of a register. */
> +#define REG_SUBREG_P(X) \
> + (REG_P (X) || SUBREG_REG_P(X))
> > * caller-save.c: Use REG_SUBREG_P and SUBREG_REG_P where applicable.
> In general, I think the introduction of predicates for common patterns
> is a good thing, so I like the idea/intent of the suggested patch.
> Now, IMVHO, having two different predicates with so similar names is a
> potential source of confusion and mistakes, for both code readers and
> code writers.
> If we're going the extra predicate way as suggested, could we please
> take the opportunity to introduce slightly more descriptive names, for
> example SUBREG_OF_REG_P and REG_OR_SUBREG_OF_REG_P ?
> I think the disambiguation-hence-mistake-avoidance is well worth
> having slightly longer names to type and read. In this particular
> case, I'd actually rather not have a second predicate instead of
> one with so similar a name.
I think that in this case the code is way easier to read if you not
have to grok all these new predicates. What is so hard about
&& REG_P (SUBREG_REG (reg))
The two names are indeed somewhat confusing.