On 23/06/2026 15:59, Richard Biener wrote:
On Tue, Jun 23, 2026 at 3:24 PM Richard Sandiford
<[email protected]> wrote:
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'.
This is simply an assignment, no different from
npatterns = get_nelts();
or
npatterns = nelts;
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".
Yeah, I think that's less confusing. OK with that change.
I'll rework the patch when I have time.
--
Christopher Bazley
Staff Software Engineer, GNU Tools Team.
Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
http://www.arm.com/
