Oleg Endo <oleg.e...@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 "()".)
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.
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...
> 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. 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.
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.
Thanks,
Richard
