On Thu, Jun 25, 2026 at 5:52 PM Christopher Bazley <[email protected]> wrote:
>
> 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:
Hmm, but diffstat doesn't say this is simpler than before. What
gimple_build_vector wants to abstract is constant vs. CTOR and
then optimize getting a gimple_val for it which requires an assignment
for a CTOR and nothing for the constant. We lack the latter thing
when not wanting to use the gimplification interface so we could split
the API to add a gimple_build_single accepting single-RHSs or vals
(don't like the name much). Much of the vectorizer already explicitly
distinguishes between constants and non-constants, possibly
since forever.
Note I didn't introduce tree_vector_builder and friends, that was
Richard S., I don't know much about when that's overkill and when
not, I assume it's always required to verify we can create a vector
when it has VLA type, and in this case we have to specify the
zero-padding of the minimum initial sequence.
I'm not against simplifying code, even when that requires new
APIs. But I don't really want to do this as part of this series
(predicated tails or making zero-padded CTORs of VLA vector
types well-defined).
> @@ -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/
