This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Use predicates for RTL objects


On Wed, Aug 07, 2019 at 01:05:51PM -0500, Segher Boessenkool wrote:
> On Wed, Aug 07, 2019 at 01:39:53PM -0400, Arvind Sankar wrote:
> > On Wed, Aug 07, 2019 at 12:33:53PM -0500, Segher Boessenkool wrote:
> > > On Wed, Aug 07, 2019 at 12:15:29PM -0400, Arvind Sankar wrote:
> > > > I would also like to get some comments on the following idea to make the
> > > > code checks more readable: I am thinking of adding
> > > > 	bool rtx_def::is_a (enum rtx_code) const
> > > > This would allow us to make all the rtx_code comparisons more readable
> > > > without having to define individual macros for each.
> > > > i.e.,
> > > > 	REG_P (x)			   => x->is_a (REG)
> > > > 	GET_CODE (x) == PLUS		   => x->is_a (PLUS)
> > > > 	GET_CODE (PATTERN (x)) == SEQUENCE => PATTERN (x)->is_a (SEQUENCE)
> > > 
> > > That makes things much worse.  Not only is it less readable (IMO), but
> > > the "is_a" idiom is used to check if something is of a certain class,
> > > which is not the case here.
> > 
> > Well, the rtx_code *is* kind of a class. It determines what fields of
> > the rtx are valid and what they contain etc.
> 
> It is not a class in the C++ sense.  Confusing this is not useful for
> anyone.

True, but the code is semantically a type identifier. Using the common
is_a idiom IMO makes it more readable for that reason, but that is a
matter of personal opinion, hence my request for comments.

> 
> > > In "GET_CODE (x) == PLUS" it is clear that what the resulting machine
> > > code does is cheap.  With "x->is_a (PLUS)", who knows what is happening
> > > below the covers!
> > 
...
> > predicate macros whose implementation is more complex than checking the
> > code field. You basically have to trust that it's sensibly implemented,
> > i.e. that it is as efficiently implemented as it can be.
> 
> That's not my point -- my point was that it is *obvious* the way things
> are now, which is nice.

My reply is pointing out that it is just as (non-)obvious with or
without that inline function, if you want to use any of the helper
macros. It wouldn't be a reasonable argument to say that INSN_P (x) is
obviously efficient while x->is_a (PLUS) is hiding some potentially
nasty things inside. There's around 200 uses of GET_CODE being compared
to PLUS in the non-target-specific code, so it's not that rare. It would
be nice to replace it with something better, but PLUS_P (x) just reads
badly to me.

> 
> > I don't think
> > people writing RTL transformations should be overly worried about what
> > machine code their predicates are generating, especially when
> > they're calling the defined API for it.
> 
> The whole *design* of RTL is based around us caring a whole lot.

I'm not saying that we don't care about performance. My point is that
if you know that what you're checking is just whether this RTX is a
PLUS-coded RTX or not, you should not care exactly which machine
instructions that check will generate, because you already know that the
test should be trivial, and any sensible implementation will be
efficient enough, especially after a compiler is done with it. i.e.
being sure of getting efficient machine code is not a reason for
avoiding macros/inline functions.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]