On Wed, Sep 30, 2026 at 4:39 PM Stefan Schulze Frielinghaus
<[email protected]> wrote:
>
> On Wed, Sep 30, 2026 at 11:26:13AM +0200, Richard Biener wrote:
> > On Tue, Sep 29, 2026 at 4:20 PM Stefan Schulze Frielinghaus
> > <[email protected]> wrote:
> > >
> > > On Tue, Sep 29, 2026 at 03:26:57PM +0200, Richard Biener wrote:
> > > > On Tue, Sep 29, 2026 at 1:32 PM Stefan Schulze Frielinghaus
> > > > <[email protected]> wrote:
> > > > >
> > > > > From: Stefan Schulze Frielinghaus <[email protected]>
> > > > >
> > > > > A defined zero value for clz/ctz does not necessarily need to coinside
> > > > > for scalar and vector modes.
> > > > >
> > > > > In vect_recog_ctz_ffs_pattern() we test whether clz/ctz has a defined
> > > > > zero value for scalars while preparing for vector modes. Similarly,
> > > > > in
> > > > > vect_recog_popcount_clz_ctz_ffs_pattern() when constructing an IFN
> > > > > call
> > > > > for a vector mode, the defined zero value for the vector mode must be
> > > > > considered and not the scalar one.
> > > > >
> > > > > With this patch we now call into target hooks
> > > > > C[LT]Z_DEFINED_VALUE_AT_ZERO with a vector mode for which targets must
> > > > > be prepared. Targets which are prepared in the sense that they can
> > > > > process vector modes because they utilize GET_MODE_UNIT_BITSIZE are
> > > > > aarch64, arm, gcn, loongarch, mips, riscv, s390.
> > > > >
> > > > > Targets which need to be adapated are i386 and rs6000 due to usage of
> > > > > GET_MODE_BITSIZE. This patch restores the previous behaviour for
> > > > > those
> > > > > targets by substituting GET_MODE_BITSIZE by GET_MODE_UNIT_BITSIZE.
> > > > >
> > > > > Targets nvptx, pru, vax use GET_MODE_BITSIZE but don't have vector
> > > > > support or in case of nvptx have no clz/ctz vector support.
> > > > >
> > > > > Targets alpha, arc, c6x, cris, csky, m68k, nds32, sparc, xtensa make
> > > > > use
> > > > > of constants only. All other targets fall back to the default
> > > > > implementation.
> > > >
> > > > I just looked at the vector pattern parts and I belive there's still a
> > > > hole in
> > > > that you ask for a specific vector mode but the vector mode being used
> > > > is eventually another, esp. when performing BB vectorization. Meaning
> > > > what's missing is a validation in vectorizable_call which should see the
> > > > final vector type/mode being used - but then the choice from pattern
> > > > recognition has to be visible as part of the internal function call -
> > > > I'm not
> > > > sure it is?
> > > >
> > > > That said, the assumption was probably the behavior would be consistent
> > > > with scalar/vector modes for a single target ...
> > >
> > > Uh bear with me; I have obviously no clue about the vectorizer. While
> > > reading the old code I got the impression that the vector type derived
> > > from the scalar type is already assumed to be correct since we also use
> > > that type for other checks like direct_internal_fn_supported_p. If I
> > > read you correctly, then this is not the case but why then use the type
> > > for direct_internal_fn_supported_p? It probably does not hurt too much
> > > since we could finally bail out in vectorizable_call in case the vector
> > > type changes in the meantime.
> > >
> > > I'm wondering when and how the type could change. Could it be the case
> > > that during pattern recog we ask for V2SI and afterwards decide to use
> > > V4SI (and theoretically the defined-at-zero value could be different for
> > > both modes).
> >
> > pattern recog will use the "preferred" vector mode as given by the target
> > but for example basic-block vectorization will look for a vector type
> > matching
> > the number of scalar lanes we have. So yes, a pattern-time V4SI could
> > become V2SI or V8SI at the time vectorizable_call is invoked. Sure, we
> > can hope defined-at-zero value is at least consistent for different vector
> > modes, but well - seems we assumed it's the same for scalar and vector
> > and were wrong for that ...
>
> Thanks for shedding some light on this. That makes sense, now. It is
> probably time to familiarize myself with the vectorizer.
>
> >
> > Anyway, I think your patch is an improvement but it likely doesn't fix
> > it in full given we emit pattern compensation code for the specific
> > defined-at-zero
> > case detected (now from the vector type). But what I was asking is whether
> > we can infer the pattern-time detected variant from the pattern .CTZ/CLZ
> > call?
> > ISTR there's a 2nd argument that specifies the value at zero - do we
> > fill that in?
> > We could verify that against the hooks value for the actual vector mode used
> > in vectorizable_call and bail if it does not match. As validation such
> > checking
> > should correctly detect the mismatch between scalar and vector for s390 and
> > refuse to vectorize wrongly?
>
> Agreed. Luckily during pattern recog we only consider the scalar
> defined-at-zero value and pass this on in the second (artificial)
> argument. Therefore, as you suggested by checking during
> vectorizable_call we could simply bail out:
>
> diff --git a/gcc/tree-vect-stmts.cc b/gcc/tree-vect-stmts.cc
> index 244f319d0e4..f7b941133cc 100644
> --- a/gcc/tree-vect-stmts.cc
> +++ b/gcc/tree-vect-stmts.cc
> @@ -3549,6 +3549,20 @@ vectorizable_call (vec_info *vinfo,
> return false;
> }
>
> + /* During vec-patterns we consider and remember the scalar defined-at-zero
> + value in the second (artifical) argument for .CLZ/.CTZ. Now, once we
> have
> + the final vector type, verify that the defined-at-zero value for the
> + vector mode coincides with the scalar one. If not, bail out. */
> + if (clz_ctz_arg1)
> + {
> + int val;
> + int d = (cfn == CFN_CLZ
> + ? CLZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vectype_in), val)
> + : CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vectype_in), val));
> + if (d != 2 || val != tree_to_shwi (clz_ctz_arg1))
> + return false;
> + }
> +
Yes, that's along the line I was thinking.
> if (VECTOR_BOOLEAN_TYPE_P (vectype_out)
> != VECTOR_BOOLEAN_TYPE_P (vectype_in))
> {
>
> This should solve the correctness concern.
>
> Utilizing match.pd for the optimization even catches more cases as
> before:
>
> diff --git a/gcc/match.pd b/gcc/match.pd
> index 688357f0ecd..b2cd69f7b63 100644
> --- a/gcc/match.pd
> +++ b/gcc/match.pd
> @@ -11764,8 +11764,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> }
> (if (ifn == IFN_CLZ && wi::to_widest (@2) == val)
> (IFN_CLZ @3 @2)))))
> -(simplify
> - (cond (ne @0 integer_zerop@1) (IFN_CLZ (convert?@3 @0) INTEGER_CST@2) @2)
> +(for cnd (cond vec_cond)
> + (simplify
> + (cnd (ne @0 integer_zerop@1) (IFN_CLZ (convert?@3 @0) INTEGER_CST@2)
> uniform_integer_cst_p@4)
Seeing this, we might want to document, in ifn.texi, that the optional
at-zero value is always scalar
(ifn.texi currently does not document that argument at all)
> (with { int val;
> internal_fn ifn = IFN_LAST;
> if (BITINT_TYPE_P (TREE_TYPE (@3)))
> @@ -11774,9 +11775,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> OPTIMIZE_FOR_BOTH))
> ifn = IFN_CLZ;
> }
> - (if (ifn == IFN_CLZ)
> - (IFN_CLZ @3 @2))))
> -
> + (if (ifn == IFN_CLZ && tree_int_cst_equal (@2, uniform_integer_cst_p
> (@4)))
> + (IFN_CLZ @3 @2)))))
> /* a != 0 ? CTZ(a) : CST -> .CTZ(a) where CST is the result of the internal
> function for 0. */
> (for func (CTZ)
> (simplify
> @@ -11803,8 +11803,9 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> }
> (if (ifn == IFN_CTZ && wi::to_widest (@2) == val)
> (IFN_CTZ @3 @2)))))
> -(simplify
> - (cond (ne @0 integer_zerop@1) (IFN_CTZ (convert?@3 @0) INTEGER_CST@2) @2)
> +(for cnd (cond vec_cond)
> + (simplify
> + (cnd (ne @0 integer_zerop@1) (IFN_CTZ (convert?@3 @0) INTEGER_CST@2)
> uniform_integer_cst_p@4)
> (with { int val;
> internal_fn ifn = IFN_LAST;
> if (BITINT_TYPE_P (TREE_TYPE (@3)))
> @@ -11813,8 +11814,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> OPTIMIZE_FOR_BOTH))
> ifn = IFN_CTZ;
> }
> - (if (ifn == IFN_CTZ)
> - (IFN_CTZ @3 @2))))
> + (if (ifn == IFN_CTZ && tree_int_cst_equal (@2, uniform_integer_cst_p
> (@4)))
> + (IFN_CTZ @3 @2)))))
> #endif
>
> /* Common POPCOUNT/PARITY simplifications. */
>
> This even matches for functions ctzv16qi_z13 and ctzv8hi_z13 from the
> previous patch. During vect-patterns this was not optimized due to the
> difference in output types and extension.
>
> Note, this is completely untested and I will be AFK for the next few
> days. So response might be delayed on my end.
>
> Cheers,
> Stefan
>
> >
> > Richard.
> >
> > >
> > > Cheers,
> > > Stefan
> > >
> > > >
> > > > > gcc/ChangeLog:
> > > > >
> > > > > * config/i386/i386.h (CTZ_DEFINED_VALUE_AT_ZERO): Utilize
> > > > > GET_MODE_UNIT_BITSIZE instead of GET_MODE_BITSIZE in order to
> > > > > deal with vector modes.
> > > > > * config/rs6000/rs6000.h (CLZ_DEFINED_VALUE_AT_ZERO): Ditto.
> > > > > (CTZ_DEFINED_VALUE_AT_ZERO): Ditto.
> > > > > * tree-vect-patterns.cc (vect_recog_ctz_ffs_pattern): Use
> > > > > defined zero value for vector modes.
> > > > > (vect_recog_popcount_clz_ctz_ffs_pattern): Test for a defined
> > > > > zero value for vector modes.
> > > > > ---
> > > > > gcc/config/i386/i386.h | 4 ++--
> > > > > gcc/config/rs6000/rs6000.h | 4 ++--
> > > > > gcc/tree-vect-patterns.cc | 22 ++++++++++------------
> > > > > 3 files changed, 14 insertions(+), 16 deletions(-)
> > > > >
> > > > > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h
> > > > > index 47725c56090..35563a6144a 100644
> > > > > --- a/gcc/config/i386/i386.h
> > > > > +++ b/gcc/config/i386/i386.h
> > > > > @@ -3060,9 +3060,9 @@ extern void debug_dispatch_window (int);
> > > > > /* The value at zero is only defined for the BMI instructions
> > > > > LZCNT and TZCNT, not the BSR/BSF insns in the original isa. */
> > > > > #define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
> > > > > - ((VALUE) = GET_MODE_BITSIZE (MODE), TARGET_BMI ? 2 : 0)
> > > > > + ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), TARGET_BMI ? 2 : 0)
> > > > > #define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
> > > > > - ((VALUE) = GET_MODE_BITSIZE (MODE), TARGET_LZCNT ? 2 : 0)
> > > > > + ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), TARGET_LZCNT ? 2 : 0)
> > > > >
> > > > >
> > > > > /* Flags returned by ix86_get_callcvt (). */
> > > > > diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> > > > > index bcd4c39d24d..98952f11189 100644
> > > > > --- a/gcc/config/rs6000/rs6000.h
> > > > > +++ b/gcc/config/rs6000/rs6000.h
> > > > > @@ -1749,14 +1749,14 @@ typedef struct rs6000_args
> > > > >
> > > > > /* The cntlzw and cntlzd instructions return 32 and 64 for input of
> > > > > zero. */
> > > > > #define CLZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE) \
> > > > > - ((VALUE) = GET_MODE_BITSIZE (MODE), 2)
> > > > > + ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
> > > > >
> > > > > /* The CTZ patterns that are implemented in terms of CLZ return -1
> > > > > for input of
> > > > > zero. The hardware instructions added in Power9 and the
> > > > > sequences using
> > > > > popcount return 32 or 64. */
> > > > > #define CTZ_DEFINED_VALUE_AT_ZERO(MODE, VALUE)
> > > > > \
> > > > > (TARGET_CTZ || TARGET_POPCNTD
> > > > > \
> > > > > - ? ((VALUE) = GET_MODE_BITSIZE (MODE), 2)
> > > > > \
> > > > > + ? ((VALUE) = GET_MODE_UNIT_BITSIZE (MODE), 2)
> > > > > \
> > > > > : ((VALUE) = -1, 2))
> > > > >
> > > > > /* Specify the machine mode that pointers have.
> > > > > diff --git a/gcc/tree-vect-patterns.cc b/gcc/tree-vect-patterns.cc
> > > > > index 5fcc37f053c..38f65c0dafa 100644
> > > > > --- a/gcc/tree-vect-patterns.cc
> > > > > +++ b/gcc/tree-vect-patterns.cc
> > > > > @@ -1918,7 +1918,7 @@ vect_recog_ctz_ffs_pattern (vec_info *vinfo,
> > > > > stmt_vec_info stmt_vinfo,
> > > > > {
> > > > > ifnnew = IFN_CTZ;
> > > > > defined_at_zero_new
> > > > > - = CTZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE (rhs_type),
> > > > > + = CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_rhs_type),
> > > > > val_new) == 2;
> > > > > }
> > > > > else if (direct_internal_fn_supported_p (IFN_CLZ, vec_rhs_type,
> > > > > @@ -1926,7 +1926,7 @@ vect_recog_ctz_ffs_pattern (vec_info *vinfo,
> > > > > stmt_vec_info stmt_vinfo,
> > > > > {
> > > > > ifnnew = IFN_CLZ;
> > > > > defined_at_zero_new
> > > > > - = CLZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE (rhs_type),
> > > > > + = CLZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_rhs_type),
> > > > > val_new) == 2;
> > > > > }
> > > > > if ((ifnnew == IFN_LAST
> > > > > @@ -2219,6 +2219,10 @@ vect_recog_popcount_clz_ctz_ffs_pattern
> > > > > (vec_info *vinfo,
> > > > > if (TYPE_PRECISION (unprom_diff.type) != TYPE_PRECISION (lhs_type))
> > > > > return NULL;
> > > > >
> > > > > + vec_type = get_vectype_for_scalar_type (vinfo, lhs_type);
> > > > > + if (!vec_type)
> > > > > + return NULL;
> > > > > +
> > > > > /* Also A should be unsigned or same precision as temp_in,
> > > > > otherwise
> > > > > different builtins/internal functions have different behaviors.
> > > > > */
> > > > > if (TYPE_PRECISION (unprom_diff.type)
> > > > > @@ -2248,8 +2252,7 @@ vect_recog_popcount_clz_ctz_ffs_pattern
> > > > > (vec_info *vinfo,
> > > > > int val1, val2;
> > > > > val1 = tree_to_shwi (gimple_call_arg (call_stmt, 1));
> > > > > int d2
> > > > > - = CLZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE
> > > > > (lhs_type),
> > > > > - val2);
> > > > > + = CLZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_type),
> > > > > val2);
> > > > > if (d2 != 2 || val1 != val2 + addend)
> > > > > return NULL;
> > > > > }
> > > > > @@ -2264,8 +2267,7 @@ vect_recog_popcount_clz_ctz_ffs_pattern
> > > > > (vec_info *vinfo,
> > > > > int val1, val2;
> > > > > val1 = tree_to_shwi (gimple_call_arg (call_stmt, 1));
> > > > > int d2
> > > > > - = CTZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE
> > > > > (lhs_type),
> > > > > - val2);
> > > > > + = CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_type),
> > > > > val2);
> > > > > if (d2 != 2 || val1 != val2)
> > > > > return NULL;
> > > > > }
> > > > > @@ -2277,11 +2279,7 @@ vect_recog_popcount_clz_ctz_ffs_pattern
> > > > > (vec_info *vinfo,
> > > > > gcc_unreachable ();
> > > > > }
> > > > >
> > > > > - vec_type = get_vectype_for_scalar_type (vinfo, lhs_type);
> > > > > /* Do it only if the backend has popcount<vector_mode>2 etc.
> > > > > pattern. */
> > > > > - if (!vec_type)
> > > > > - return NULL;
> > > > > -
> > > > > bool supported
> > > > > = direct_internal_fn_supported_p (ifn, vec_type,
> > > > > OPTIMIZE_FOR_SPEED);
> > > > > /* We cannot rely on the reduction check in
> > > > > vect_recog_ctz_ffs_pattern
> > > > > @@ -2334,11 +2332,11 @@ vect_recog_popcount_clz_ctz_ffs_pattern
> > > > > (vec_info *vinfo,
> > > > > tree arg2 = NULL_TREE;
> > > > > int val;
> > > > > if (ifn == IFN_CLZ
> > > > > - && CLZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE (lhs_type),
> > > > > + && CLZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_type),
> > > > > val) == 2)
> > > > > arg2 = build_int_cst (integer_type_node, val);
> > > > > else if (ifn == IFN_CTZ
> > > > > - && CTZ_DEFINED_VALUE_AT_ZERO (SCALAR_INT_TYPE_MODE
> > > > > (lhs_type),
> > > > > + && CTZ_DEFINED_VALUE_AT_ZERO (TYPE_MODE (vec_type),
> > > > > val) == 2)
> > > > > arg2 = build_int_cst (integer_type_node, val);
> > > > > if (arg2)
> > > > > --
> > > > > 2.55.0
> > > > >
