Hi Richard,
On 24/06/2026 14:39, Richard Biener wrote:
On Wed, Jun 24, 2026 at 2:11 PM Richard Sandiford
<[email protected]> wrote:
Richard Biener <[email protected]> writes:
Not sure whether this is how you understood it, but when I said:
The only valid situations seem to be:
(1) a duplicate of a single zero, where:
npatterns == nelts_per_pattern == encoded_nelts == 1
and the only encoded value is zero
(2) the combination of:
- nelts_per_pattern == 2
- multiple_p (TYPE_VECTOR_SUBPARTS (type), npatterns)
- the second half of the encoded elements are all zeros
I meant that those conditions seemed to be the ones that your code would
need to follow in order to fill VLA vectors with zeros. Those conditions
certainly don't apply to all gimple_build_vector callers.
So to be correct, the original patch would need to add an assert to
gimple_build_vector that checks the above conditions before using
constant_lower_bound. Like you say above, without the assert:
{ 1, x } nelts_per_pattern == 1, npatterns == 2
would incorrectly give:
{ 1, x, 0, 0, 0, 0, ... }
whereas it should instead be:
{ 1, x, 1, x, 1, x, ... }
The current VLS CONSTRUCTOR path is supposedly correct because it
explicitly initialises every element of the vector (i.e. it does not
rely on zero padding of the constructor).
My argument was that (1) or (2) above would not come about by chance.
The caller would have to do something explicit to ensure that (2) is true.
And if (1) or (2) is true, there is no need to for the tree_vector_builder.
An array of the leading nonzero elements would be good enough, and would
be simpler to build.
That's why I thought that a different interface from the tree_vector_builder
version would be better than adding a complicated assert to the existing
function.
My argument is that the current API can cover the new use just fine and
we should avoid introducing new APIs for a single use which could then
well just build a properly constrained CTOR directly. As we've legitimized
the zero-padded VLA typed CTOR variant it is IMHO sensible to have
gimple_build_vector support that case.
Did I say we have too many APIs already?
Many APIs start out as single use though.
The reason for having two APIs in this case is that ctors and vector
constants have different representations. The current API matches
the vector constant representation and the new one would match the
ctor representation.
But maybe the two representations are the real issue, and that ctors
should have the same representation as constants and be built in the
same way as constants (although that might be significantly more awkward
for users).
I think the real issue is that tree_vector_builder isn't the most
suitable representation for every interface. It is more expressive than
most callers need. Expressivity is not always a good thing.
Just because any infinite-length sequence of elements (including one
ending in an infinite number of zeros) can be encoded as a
tree_vector_builder, it does not follow that tree_vector_builder is
always the appropriate choice. For example, consider the fact that
duplicate_and_interleave is always called with a tree_vector_builder,
but that tree_vector_builder is in fact treated as a const vec<tree>
because of subtype polymorphism (thereby quietly losing its shape).
I don't see why duplicate_and_interleave should be permitted to consume
a vec<tree> but equivalent functions on alternative paths should not.
From the point of view of clarity and simplicity, it seems better to me
that data never has a shape in the first place, instead of having a
shape that is either discarded or not properly honoured.
There seem to be only two call sites at which representing the input to
gimple_build_vector as a tree_vector_builder is actually necessary (in
get_initial_defs_for_reduction and vect_create_constant_vectors); in
every other case, a repeating sequence is not needed. Such a sequence
would be accidental if it ever occurred, and permitting it
introduces tricky and irrelevant failure modes
(three-element-per-pattern encodings not supported at all,
two-element-per-pattern encodings only supported for VLA types if every
second element is zero).
To me, the choice to deny the majority of callers a simpler interface
that is better aligned with their needs seems to me to impose
unnecessary complexity.
It's also why...
The only practical thing that my gimple_build_vector_from_elems function
does differently from gimple_build_vector for non-constant vectors is
that it does not rely on the vector type having a fixed length: instead,
it relies on implicit zero-filling of not-mentioned elements of a
CONSTRUCTOR node. Is that behaviour you would be willing to adopt in
gimple_build_vector?
...I don't think that this question really applies (assuming that
you're asking about the current gimple_build_vector). By design,
tree_vector_builder implicitly specifies a full vector's worth of
elements based on a possibly shorter sequence of explicitly elements.
There are no ambiguities to be resolved. There is nothing that is left
to gimple_build_vector's interpretation.
If we don't want tree_vector_builder semantics then we should provide
an interface that doesn't use tree_vector_builder, which is essentially
my argument above.
But we have that. Build a CTOR and then an assignment.
But I thought the idea of the gimple_build_* interfaces was to avoid
unnecessary assignments. I.e. the purpose of the new API would be to
fold a ctor to a constant without ever creating a temporary SSA name.
True, but then this is valid for the VLA "head" case as well. I've
asked for how he handles constants there (should just zero-pad).
Anyway, I suppose I should bow out of this. I'll have wasted a lot of
Chris's time by suggesting the path that I did.
I don't have any ill feelings about it.
I do think though that the ctor path in gimple_build_vector should
check for the case that it handles (i.e. zero padding), rather than
just ignoring the second half of the tree_vector_builder and hoping
for the best.
Yes, of course - that's what I was asking for.
I will try to create a patch that does what you want, but in the
meantime I have been experimenting with addressing one of your points
against my gimple_build_vector_from_elems function: that "we should
avoid introducing new APIs for a single use which could then well just
build a properly constrained CTOR directly".
The CTOR is not built directly by callers of gimple_build_vector, so
from my point of view, the only question is how far building of that
CTOR should be delegated down the call stack. I think the answer should
depend on the needs of the calling code.
I don't think a function resembling gimple_build_vector_from_elems does
need to have only a single use. As I observed above, there are many
existing callers of gimple_build_vector for which repetition is not
required, including calls where logic that makes my brain hurt is needed
to ensure that gimple_build_vector is not called inappropriately, and
that it has the expected behaviour if it is called.
Despite that guarding logic, the caller still has to rely on assumptions
that are unchecked at the call site and cannot easily be statically
verified, such as "number of elements per pattern is not three" and "if
number of elements per pattern is two then the second of each pattern is
constant zero... unless the builder has a fixed-length vector type".
Here's an example of how I believe I have simplified one of the more
complex existing uses of gimple_build_vector:
@@ -4986,57 +4985,63 @@ get_initial_defs_for_reduction (loop_vec_info
loop_vinfo,
initial_values[i]);
}
op = initial_values[i];
}
/* Create 'vect_ = {op0,op1,...,opn}'. */
number_of_places_left_in_vector--;
elts[nunits - number_of_places_left_in_vector - 1] = op;
- if (!CONSTANT_CLASS_P (op))
- constant_p = false;
if (number_of_places_left_in_vector == 0)
{
- tree init;
- if (constant_p && !neutral_op
- ? multiple_p (TYPE_VECTOR_SUBPARTS (vector_type), nunits)
- : known_eq (TYPE_VECTOR_SUBPARTS (vector_type), nunits))
- /* Build the vector directly from ELTS. */
- init = gimple_build_vector (&ctor_seq, &elts);
+ tree init = NULL_TREE;
+ if (known_eq (TYPE_VECTOR_SUBPARTS (vector_type), nunits))
+ {
+ /* No repetition or neutral value is needed. */
+ init = gimple_build_simple_vector (&ctor_seq, vector_type,
elts);
+ gcc_checking_assert (init);
+ }
else if (neutral_op)
{
/* Build a vector of the neutral value and shift the
other elements into place. */
init = gimple_build_vector_from_val (&ctor_seq, vector_type,
neutral_op);
int k = nunits;
while (k > 0 && operand_equal_p (elts[k - 1], neutral_op))
k -= 1;
while (k > 0)
{
k -= 1;
init = gimple_build (&ctor_seq, CFN_VEC_SHL_INSERT,
vector_type, init, elts[k]);
}
+ gcc_checking_assert (init);
}
- else
+ else if (multiple_p (TYPE_VECTOR_SUBPARTS (vector_type), nunits))
+ {
+ /* Repeat ELTS to fill the vector. This can fail to build
+ a CONSTRUCTOR node if there are non-constant elements
+ and VECTOR_TYPE is variable-length. */
+ init = gimple_try_build_repeating_vector (&ctor_seq,
vector_type,
+ elts);
+ }
+
+ if (!init)
{
/* First time round, duplicate ELTS to fill the
required number of vectors. */
duplicate_and_interleave (loop_vinfo, &ctor_seq,
vector_type, elts,
number_of_vectors, *vec_oprnds);
break;
}
vec_oprnds->quick_push (init);
number_of_places_left_in_vector = nunits;
- elts.new_vector (vector_type, nunits, 1);
- elts.quick_grow (nunits);
- constant_p = true;
}
}
if (ctor_seq != NULL)
vect_emit_reduction_init_stmts (loop_vinfo, reduc_info, ctor_seq);
}
vect_reduc_info
info_for_reduction (loop_vec_info loop_vinfo, slp_tree node)
I'm not trying to maintain the terseness of the original code; I'm
trying to make it easy to understand and maintain. I hope you will
reconsider the possible merits of this approach when I email the next
version of my patch.
Another reason for providing separate gimple_build_simple_vector and
gimple_try_build_repeating_vector functions is that it would potentially
allow the upper lanes of a vector created by gimple_build_simple_vector
to be treated as "don't care" in future, without requiring all call
sites to be updated.
Like I wrote, I'll also work on trying to implement your suggested approach.
Thanks,
--
Christopher Bazley
Staff Software Engineer, GNU Tools Team.
Arm Ltd, 110 Fulbourn Road, Cambridge, CB1 9NJ, UK.
http://www.arm.com/
