On 3/11/20 11:44 AM, Marek Polacek wrote:
On Wed, Mar 11, 2020 at 07:52:15AM +0100, Jakub Jelinek via Gcc-patches wrote:
On Tue, Mar 10, 2020 at 07:38:17PM -0400, Marek Polacek via Gcc-patches wrote:
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6062,6 +6062,13 @@ reshape_init_array_1 (tree elt_type, tree max_index, 
reshape_iter *d,
        else if (last_nonzero < nelts - 1)
        nelts = last_nonzero + 1;
+ /* Sharing a stripped constructor can get in the way of
+        overload resolution.  E.g., initializing a class from
+        {{0}} might be invalid while initializing the same class
+        from {{}} might be valid.  */
+      if (reuse)
+       new_init = unshare_constructor (new_init);
+
        vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);

Isn't it wasteful to first copy perhaps a large constructor (recursively)
and then truncate it to very few elts (zero in this case)?
So, perhaps doing instead:
       if (reuse)
        {
          vec<constructor_elt, va_gc> *v = NULL;
          if (nelts)
            vec_alloc (v, nelts);
          for (unsigned int i = 0; i < nelts; i++)
            {
              constructor_elt elt = CONSTRUCTOR_ELT (new_init, i);
              if (TREE_CODE (elt.value) == CONSTRUCTOR)
                elt.value = unshare_constructor (elt.value);
              v->quick_push (elt);
            }
          new_init = build_constructor (TREE_TYPE (new_init), v);
        }
       else
        vec_safe_truncate (CONSTRUCTOR_ELTS (new_init), nelts);
?

Large constructors aren't expected here.  From bootstrap + testsuite
measurements I can see that we have 8035 actual truncations, and 7884
of those are of constructors shorter than 10 elements.  There's one case
where we have a ctor of 567 elements and truncate it to 559 elements but
that seems pathological.  Is it worth it (genuine quiestion)?

Note there are 37077 attempts to truncate, of which 29042 truncate to
the same length.  Our ->truncate merely sets m_num, so it's cheap, but
I'm still a bit surprised we don't check

   if (CONSTRUCTOR_NELTS (new_init) != nelts)

before calling vec_safe_truncate.

We should certainly avoid copying if they're the same. The code above for only copying the bits that aren't going to be thrown away seems pretty straightforward, might as well use it even if the savings aren't likely to be large.

Jason

Reply via email to