Re: Instructions vs Expressions in the backend (was Re: RFA: Rework FOR_BB_INSNS iterators)

[Oleg Endo <oleg dot endo at t-online dot de>]

On Wed, 2014-06-25 at 10:36 +0100, Richard Sandiford wrote:
> Oleg Endo <oleg.endo@t-online.de> writes:
> > Personally, I'd like to see usage of standard STL-like iterator usage.
> > I've proposed something for edge_iterator a while ago, but people don't
> > seem very fond of it.  See also
> > https://gcc.gnu.org/ml/gcc-patches/2013-12/msg01129.html
> >
> > Have you also considered passing the new rtx_* types by value or
> > reference instead of pointer?  A long time ago I've quickly put together
> > a class 'rtxx' which was just a pointer wrapper for the rtx_def*
> > (basically the same as I proposed for edge_iterator).
> > I've converted the SH backend code to use it just to see what it would
> > look like.  The conversion itself was pretty straight forward -- just
> > replace 'rtx' with 'rtxx'.  Appropriate conversion
> > operators/constructors in 'class rtxx' made both interchangeable and
> > allowed co-existence of both and thus step-by-step conversion of the
> > code base.
> > Another advantage of passing around by value/ref is that it allows
> > operator overloading.  One use case could be instead of:
> >
> > if (MEM_P (XEXP (x, 0)))
> >   *total = address_cost (XEXP (XEXP (x, 0), 0),
> >                          GET_MODE (XEXP (x, 0)),
> >                          MEM_ADDR_SPACE (XEXP (x, 0)), true);
> >
> >
> > something like that (overloading operator[]):
> > if (x[0] == rtx_mem::type)
> >   *total = address_cost (x[0][0], x[0].mode (),
> >                          x[0].mem_addr_space (), true);
> >
> > ... where rtx_mem::type would be some type for which 'rtxx' (or whatever
> > the name of the base class is) would provide the according operator
> > ==, != overloads.
> 
> I think this is an example of another problem with gcc coding style:
> that we're far too afraid of temporary variables.  In David's scheme
> I think this would be:
> 
>   if (rtx_mem *mem = as_a <rtx_mem *> (XEXP (x, 0)))
>     *total = address_cost (XEXP (mem, 0), GET_MODE (mem),
>                            MEM_ADDR_SPACE (mem), true);
> 
> which with members would become:
> 
>   if (rtx_mem *mem = as_a <rtx_mem *> (...))
>     *total = address_cost (mem->address (), mem->mode (), mem->address_space (),
>                            true);
> 
> (although if we go down that route, I hope we can add an exception to the
> formatting rule so that no space should be used before "()".)

If such an exception is introduced, I can imagine it'd be difficult to
judge on a case by case whether to apply the exceptional rule or not,
since those are just function calls.
But in the end it doesn't matter.  It's just a matter of habbit :)

> 
> I suppose with the magic values it would be:
> 
>   if (rtx_mem mem = as_a <rtx_mem> (x[0]))
>     *total = address_cost (mem[0], mem.mode (), mem.address_space (), true);
> 
> but I'm not sure that that would really be more readable.

No, essentially it's the same.  Whether the current XEXP (x, n) is done
by operator [] or a member function 'xexp (n)' doesn't matter.  The
biggest (optical) change is for nested XEXP...
    XEXP (XEXP (XEXP (x, a), b), c)
becomes
    x.exp (a).exp (b).exp (c)

> FWIW I also did an experiment locally with replacing "rtx *" (rather than
> "rtx") with a "smart" pointer so that we could have four allocation
> areas: permanent, gty, function and temporary, with the pointer
> automatically promoting rtxes to the right allocation area for the
> containing object.
> 
> Having a smart pointer made it suprisingly uninvasive but there was
> probably too much C++ magic involved in the handling of XEXP, etc.,
> for the patch to be acceptable.  Still, it did noticeably reduce max RSS
> and was a significant compile-time saving for extreme compiles like
> insn-recog.ii.  Hope to return to it sometime...

Yep, passing the new rtx classes by value instead by pointer opens other
doors, such as implementing smart pointers internally and what not. 

> 
> > Another thing is rtx construction in C++ code, which could look like:
> >
> > emit_insn (rtx_insn (rtx_set (rtx_reg (0),
> >                               rtx_plus (rtx_reg (1), rtx_reg (2)))));
> > emit_insn (rtx_barrier ());
> >
> > Although I'm not sure how often this is needed in current practice,
> > since most of the time rtx instances are created from the .md patterns.
> > Maybe it could be useful for implementing some kind of ad-hoc rtx
> > matching, as it's found in cost calculations, predicates, constrants,
> > e.g.
> >
> > if ((GET_CODE (XEXP (x, 0)) == SMAX || GET_CODE (XEXP (x, 0)) == SMIN)
> >     && CONST_INT_P (XEXP (XEXP (x, 0), 1))
> >     && REG_P (XEXP (XEXP (x, 0), 0))
> >     && CONST_INT_P (XEXP (x, 1)))
> >
> > could become:
> > if (matches (x, rtx_smax (reg_rtx (), const_int (), const_int ()))
> >     || matches (x, rtx_smin (reg_rtx (), const_int (), const_int ()))
> >
> > somehow I find that easier to read and write.
> 
> It sounds like this would be creating temporary rtxes though, which would
> be too expensive for matching.

Yeah, as I wrote it up there, probably.  It depends on the
implementation of those default constructed objects.

>   Maybe it would make sense to have a separate
> set of matching objects that only live for the duration of the statement.
> Then you could have matching objects like "range (min, max)" to match a
> range of integers.

Maybe something like this ...

static my_match_rtx = rtx_smax (reg_rtx (), const_int (), const_int ());
if (matches (x, my_match_rtx) ... 


> But like you say, I'm not sure whether it would really be a win in the end.
> I think what makes this example so hard to read is again that we refuse
> to put XEXP (x, 0) in a temporary variable and just write it out in full
> 4 times instead.
> 
>   if ((GET_CODE (op0) == SMAX || GET_CODE (op0) == SMIN)
>       && CONST_INT_P (XEXP (op0, 1))
>       && REG_P (XEXP (op0, 0))
>       && CONST_INT_P (op1))
> 
> is a bit more obvious.

In this case yes.  On the other hand, that if statement is part of a
bigger function.  There are cases, where manual CSE is not so practical,
e.g. when it's an if - else if - else if - else.

Cheers,
Oleg