On 1/21/25 5:02 PM, Jakub Jelinek wrote:
On Tue, Jan 21, 2025 at 04:39:58PM -0500, Jason Merrill wrote:
On 1/21/25 11:15 AM, Jakub Jelinek wrote:
On Tue, Jan 21, 2025 at 11:06:35AM -0500, Jason Merrill wrote:
--- gcc/c-family/c-common.cc.jj 2025-01-20 18:00:35.667875671 +0100
+++ gcc/c-family/c-common.cc 2025-01-21 09:29:23.955582581 +0100
@@ -9010,33 +9010,46 @@ make_tree_vector_from_list (tree list)
return ret;
}
-/* Get a new tree vector of the values of a CONSTRUCTOR. */
+/* Append to a tree vector the values of a CONSTRUCTOR.
+ nelts should be at least CONSTRUCTOR_NELTS (ctor) and v
+ should be initialized with make_tree_vector (); followed by
+ vec_safe_reserve (v, nelts); or equivalently vec_alloc (v, nelts);
+ optionally followed by pushes of other elements (up to
+ nelts - CONSTRUCTOR_NELTS (ctor)). */
How about using v->allocated () instead of passing in nelts?
That is not necessarily the same.
Yeah, it occurred to me later that it doesn't matter what the original
length or capacity of the vector is, we want to make sure there's enough
room for the elements of the ctor after whatever's already there. So we
want nelts to start as CONSTRUCTOR_NELTS, and then vec_safe_reserve nelts -
i.
That wouldn't work for the appending case, the vector already can have some
extra elements in it.
But actually starting at
unsigned nelts = vec_safe_length (v) + CONSTRUCTOR_NELTS (ctor);
is I think exactly what we want. And I think we should keep the
vec_safe_reserve or vec_alloc in the callers unless there is a RAW_DATA_CST,
CONSTRUCTORs without those will still be the vast majority of cases and for
GC vectors reservation when there aren't enough allocated elements means new
allocation and GC of the old.
In the make_tree_vector_from_ctor case that will be 0 + CONSTRUCTOR_NELTS
(ctor);
and caller reserving (non-exact) that amount, while in the case of
add_list_candidates it starts from what we have already in the vector (i.e.
the nart extra first args) and then CONSTRUCTOR_NELTS (ctor) again.
And vec_safe_length (v) rather than v->length (), because in the
add_list_candidates case for nart == 0 && CONSTRUCTOR_NELTS (ctor) == 0
vec_alloc (new_args, 0);
just sets new_args = NULL (in the make_tree_vector_from_ctor case
make_tree_vector () actually returns some vector with allocated () in
[4,16]).
So like the patch below (again, just quickly tested on a few tests so far)?
2025-01-21 Jakub Jelinek <ja...@redhat.com>
gcc/c-family/
* c-common.h (append_ctor_to_tree_vector): Declare.
* c-common.cc (append_ctor_to_tree_vector): New function.
(make_tree_vector_from_ctor): Use it.
gcc/cp/
* call.cc (add_list_candidates): Use append_ctor_to_tree_vector.
--- gcc/c-family/c-common.h.jj 2025-01-17 11:29:33.139696380 +0100
+++ gcc/c-family/c-common.h 2025-01-21 09:30:09.520947570 +0100
@@ -1190,6 +1190,8 @@ extern vec<tree, va_gc> *make_tree_vecto
extern void release_tree_vector (vec<tree, va_gc> *);
extern vec<tree, va_gc> *make_tree_vector_single (tree);
extern vec<tree, va_gc> *make_tree_vector_from_list (tree);
+extern vec<tree, va_gc> *append_ctor_to_tree_vector (vec<tree, va_gc> *,
+ tree);
extern vec<tree, va_gc> *make_tree_vector_from_ctor (tree);
extern vec<tree, va_gc> *make_tree_vector_copy (const vec<tree, va_gc> *);
--- gcc/c-family/c-common.cc.jj 2025-01-20 18:00:35.667875671 +0100
+++ gcc/c-family/c-common.cc 2025-01-21 22:47:30.644455174 +0100
@@ -9010,33 +9010,45 @@ make_tree_vector_from_list (tree list)
return ret;
}
-/* Get a new tree vector of the values of a CONSTRUCTOR. */
+/* Append to a tree vector the values of a CONSTRUCTOR.
+ v should be initialized with make_tree_vector (); followed by
+ vec_safe_reserve (v, nelts); or equivalently vec_alloc (v, nelts);
+ optionally followed by pushes of other elements (up to
+ nelts - CONSTRUCTOR_NELTS (ctor)). */
vec<tree, va_gc> *
-make_tree_vector_from_ctor (tree ctor)
+append_ctor_to_tree_vector (vec<tree, va_gc> *v, tree ctor)
{
- vec<tree,va_gc> *ret = make_tree_vector ();
- unsigned nelts = CONSTRUCTOR_NELTS (ctor);
- vec_safe_reserve (ret, CONSTRUCTOR_NELTS (ctor));
I think we can/should still have
vec_safe_reserve (v, CONSTRUCTOR_NELTS (ctor));
here, to place fewer requirements on callers; if it's redundant it will
just return.
OK with that change, thanks.
+ unsigned nelts = vec_safe_length (v) + CONSTRUCTOR_NELTS (ctor);
for (unsigned i = 0; i < CONSTRUCTOR_NELTS (ctor); ++i)
if (TREE_CODE (CONSTRUCTOR_ELT (ctor, i)->value) == RAW_DATA_CST)
{
tree raw_data = CONSTRUCTOR_ELT (ctor, i)->value;
nelts += RAW_DATA_LENGTH (raw_data) - 1;
- vec_safe_reserve (ret, nelts - ret->length ());
+ vec_safe_reserve (v, nelts - v->length ());
if (TYPE_PRECISION (TREE_TYPE (raw_data)) > CHAR_BIT
|| TYPE_UNSIGNED (TREE_TYPE (raw_data)))
for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
- ret->quick_push (build_int_cst (TREE_TYPE (raw_data),
- RAW_DATA_UCHAR_ELT (raw_data, j)));
+ v->quick_push (build_int_cst (TREE_TYPE (raw_data),
+ RAW_DATA_UCHAR_ELT (raw_data, j)));
else
for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
- ret->quick_push (build_int_cst (TREE_TYPE (raw_data),
- RAW_DATA_SCHAR_ELT (raw_data, j)));
+ v->quick_push (build_int_cst (TREE_TYPE (raw_data),
+ RAW_DATA_SCHAR_ELT (raw_data, j)));
}
else
- ret->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
- return ret;
+ v->quick_push (CONSTRUCTOR_ELT (ctor, i)->value);
+ return v;
+}
+
+/* Get a new tree vector of the values of a CONSTRUCTOR. */
+
+vec<tree, va_gc> *
+make_tree_vector_from_ctor (tree ctor)
+{
+ vec<tree,va_gc> *ret = make_tree_vector ();
+ vec_safe_reserve (ret, CONSTRUCTOR_NELTS (ctor));
+ return append_ctor_to_tree_vector (ret, ctor);
}
/* Get a new tree vector which is a copy of an existing one. */
--- gcc/cp/call.cc.jj 2025-01-21 09:11:58.214113697 +0100
+++ gcc/cp/call.cc 2025-01-21 19:13:01.568407526 +0100
@@ -4258,30 +4258,10 @@ add_list_candidates (tree fns, tree firs
/* Expand the CONSTRUCTOR into a new argument vec. */
vec<tree, va_gc> *new_args;
- unsigned nelts = nart + CONSTRUCTOR_NELTS (init_list);
- vec_alloc (new_args, nelts);
+ vec_alloc (new_args, nart + CONSTRUCTOR_NELTS (init_list));
for (unsigned i = 0; i < nart; ++i)
new_args->quick_push ((*args)[i]);
- for (unsigned i = 0; i < CONSTRUCTOR_NELTS (init_list); ++i)
- if (TREE_CODE (CONSTRUCTOR_ELT (init_list, i)->value) == RAW_DATA_CST)
- {
- tree raw_data = CONSTRUCTOR_ELT (init_list, i)->value;
- nelts += RAW_DATA_LENGTH (raw_data) - 1;
- vec_safe_reserve (new_args, nelts - new_args->length ());
- if (TYPE_PRECISION (TREE_TYPE (raw_data)) > CHAR_BIT
- || TYPE_UNSIGNED (TREE_TYPE (raw_data)))
- for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
- new_args->quick_push (build_int_cst (TREE_TYPE (raw_data),
- RAW_DATA_UCHAR_ELT (raw_data,
- j)));
- else
- for (unsigned j = 0; j < (unsigned) RAW_DATA_LENGTH (raw_data); ++j)
- new_args->quick_push (build_int_cst (TREE_TYPE (raw_data),
- RAW_DATA_SCHAR_ELT (raw_data,
- j)));
- }
- else
- new_args->quick_push (CONSTRUCTOR_ELT (init_list, i)->value);
+ new_args = append_ctor_to_tree_vector (new_args, init_list);
/* We aren't looking for list-ctors anymore. */
flags &= ~LOOKUP_LIST_ONLY;
Jakub
