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 ...
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?
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
> > >
