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);
FWIW you want a dyn_cast<> rather than an as_a<> here, giving:
if (rtx_mem *mem = dyn_cast <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);
(likewise)
> (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);
(likewise).
> but I'm not sure that that would really be more readable.
[...snip...; see my other mail for notes on restricting the scope of the
current patch kit to an insn vs expr separation, for the sake of my
sanity :) ]