Richard Biener <[email protected]> writes:
> On Tue, Jun 23, 2026 at 12:54 PM Christopher Bazley via Sourceware
> Forge <[email protected]> wrote:
>>
>> From: Christopher Bazley <[email protected]>
>>
>> 'nelts' is not the number of encoded elements in a VECTOR_CST; it
>> is the number of patterns. Sometimes those interpretations coincide,
>> as in the case of fixed length vector types (for which the number of
>> elements per pattern is one), but not always.
>>
>> 'step' is not the step between the second and third element of
>> a stepped pattern; it is the number of elements per pattern.
>> This has led to confusion on the mailing list.
>>
>> Both variables are hereby renamed.
>>
>> gcc/ChangeLog:
>>
>> * tree.cc (build_vector_from_ctor): Rename variables.
>> ---
>> gcc/tree.cc | 16 ++++++++--------
>> 1 file changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>> index ca713cec4d6a..b4c836b4a0a7 100644
>> --- a/gcc/tree.cc
>> +++ b/gcc/tree.cc
>> @@ -2201,23 +2201,23 @@ build_vector_from_ctor (tree type, const
>> vec<constructor_elt, va_gc> *v)
>> if (vec_safe_length (v) == 0)
>> return build_zero_cst (type);
>>
>> - unsigned HOST_WIDE_INT idx, nelts, step = 1;
>> + unsigned HOST_WIDE_INT idx, npatterns, nelts_per_pattern = 1;
>> tree value;
>>
>> /* If the vector is a VLA, build a VLA constant vector. */
>> - if (!TYPE_VECTOR_SUBPARTS (type).is_constant (&nelts))
>> + if (!TYPE_VECTOR_SUBPARTS (type).is_constant (&npatterns))
>
> No, TYPE_VECTOR_SUBPARTS is the number of elements in 'type'.
Yeah, but in this context it's being used to set up the parameters
to tree_vector_builder. In other words, the name isn't describing
where the information comes from, but rather what it ends up being
used for.
The current name "nelts" works here, but is at best misleading:
>> {
>> - nelts = constant_lower_bound (TYPE_VECTOR_SUBPARTS (type));
...here, and at worst wrong.
So I thought the patch was ok, but I won't push for it if you disagree.
>> - gcc_assert (vec_safe_length (v) <= nelts);
>> - step = 2;
>> + npatterns = constant_lower_bound (TYPE_VECTOR_SUBPARTS (type));
>> + gcc_assert (vec_safe_length (v) <= npatterns);
>> + nelts_per_pattern = 2;
>> }
In case this seems more palatable, I suppose another way of writing this
would be:
npatterns = constant_lower_bound (TYPE_VECTOR_SUBPARTS (type));
nelts_per_pattern = TYPE_VECTOR_SUBPARTS (type).is_constant () ? 1 : 2;
gcc_assert (vec_safe_length (v) <= npatterns);
without the "if". Or we could just use nelts_per_pattern==2 unconditionally
(with some wasted work for VLS).
Thanks,
Richard
>>
>> - tree_vector_builder vec (type, nelts, step);
>> + tree_vector_builder vec (type, npatterns, nelts_per_pattern);
>> FOR_EACH_CONSTRUCTOR_VALUE (v, idx, value)
>> {
>> if (TREE_CODE (value) == VECTOR_CST)
>> {
>> - /* If NELTS is constant then this must be too. */
>> + /* If NPATTERNS is constant then this must be too. */
>> unsigned int sub_nelts = VECTOR_CST_NELTS (value).to_constant ();
>> for (unsigned i = 0; i < sub_nelts; ++i)
>> vec.quick_push (VECTOR_CST_ELT (value, i));
>> @@ -2225,7 +2225,7 @@ build_vector_from_ctor (tree type, const
>> vec<constructor_elt, va_gc> *v)
>> else
>> vec.quick_push (value);
>> }
>> - while (vec.length () < nelts * step)
>> + while (vec.length () < npatterns * nelts_per_pattern)
>> vec.quick_push (build_zero_cst (TREE_TYPE (type)));
>>
>> return vec.build ();
>> --
>> 2.54.0
>>
