On Wed, 1 Feb 2017, Jakub Jelinek wrote:
> On Wed, Feb 01, 2017 at 03:03:15PM +0100, Richard Biener wrote:
> > 2017-02-01 Richard Biener <rguent...@suse.de>
> >
> > PR cp/14179
> > cp/
> > * cp-gimplify.c (cp_fold): When folding a CONSTRUCTOR copy
> > it lazily on the first changed element only and copy it
> > fully upfront, only storing changed elements.
> >
> > Index: gcc/cp/cp-gimplify.c
> > ===================================================================
> > --- gcc/cp/cp-gimplify.c (revision 245094)
> > +++ gcc/cp/cp-gimplify.c (working copy)
> > @@ -2361,12 +2361,9 @@ cp_fold (tree x)
> > bool changed = false;
> > vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (x);
> > vec<constructor_elt, va_gc> *nelts = NULL;
> > - vec_safe_reserve (nelts, vec_safe_length (elts));
> > FOR_EACH_VEC_SAFE_ELT (elts, i, p)
> > {
> > tree op = cp_fold (p->value);
> > - constructor_elt e = { p->index, op };
> > - nelts->quick_push (e);
> > if (op != p->value)
> > {
> > if (op == error_mark_node)
> > @@ -2375,7 +2372,13 @@ cp_fold (tree x)
> > changed = false;
> > break;
> > }
> > - changed = true;
> > + if (! changed)
> > + {
> > + nelts = elts->copy ();
>
> Isn't the above part unnecessarily expensive, e.g. for the case
> where you have huge CONSTRUCTOR and recursive cp_fold changes already
> the very first value? Wouldn't it be better to just do:
> vec_safe_reserve (nelts, vec_safe_length (elts));
> vec_quick_grow (nelts, i);
> memcpy (nelts->address (), elts->address (),
> i * sizeof (constructor_elt));
> and then:
It really depends on how many constructor elements usually fold.
If every next element will fold then yes, otherwise a memcpy
is going to be faster than individual vec_quick_push ()s with
cache-trashing cp_fold calls inbetween.
But I didn't benchmark anything, I just looked at memory use
(for the case where nothing folds). And elts->copy () looks
much "cleaner" than this reserve/grow/memcpy ;)
So I'll do whatever Jason suggests here.
Thanks,
Richard.
