On Mon, Aug 5, 2024 at 3:04 PM Li, Pan2 <pan2...@intel.com> wrote:
>
> > Isn't that now handled by the direct_internal_fn_supported_p check? That
> > is,
> > by the caller which needs to verify the matched operation is supported by
> > the target?
>
> type_strictly_matches_mode_p doesn't help here (include the un-committed one).
> It will hit below case and return true directly as TYPE_MODE (type) is
> E_RVVM1QImode.
>
> if (VECTOR_TYPE_P (type))
> return VECTOR_MODE_P (TYPE_MODE (type));
>
> And looks we cannot TREE_PRECISION on vector type here similar as
> type_has_mode_precision_p
> do for scalar types. Thus, add the check to the matching.
>
> Looks like we need to take care of vector in type_strictly_matches_mode_p,
> right ?
Well that means the caller (vectorizer pattern recog?) wrongly used a
vector of QImode in
the first place, so it needs to check the scalar mode as well? Vector
type assignment does
/* For vector types of elements whose mode precision doesn't
match their types precision we use a element type of mode
precision. The vectorization routines will have to make sure
they support the proper result truncation/extension.
We also make sure to build vector types with INTEGER_TYPE
component type only. */
if (INTEGRAL_TYPE_P (scalar_type)
&& (GET_MODE_BITSIZE (inner_mode) != TYPE_PRECISION (scalar_type)
|| TREE_CODE (scalar_type) != INTEGER_TYPE))
scalar_type = build_nonstandard_integer_type (GET_MODE_BITSIZE (inner_mode),
TYPE_UNSIGNED (scalar_type));
So possibly vectorizable_internal_function would need to be amended or better,
vector pattern matching be constrainted.
Richard.
> Pan
>
> -----Original Message-----
> From: Richard Biener <richard.guent...@gmail.com>
> Sent: Monday, August 5, 2024 7:02 PM
> To: Li, Pan2 <pan2...@intel.com>
> Cc: gcc-patches@gcc.gnu.org; juzhe.zh...@rivai.ai; kito.ch...@gmail.com;
> jeffreya...@gmail.com; rdapp....@gmail.com
> Subject: Re: [PATCH v1] Match: Add type_has_mode_precision_p check for
> SAT_TRUNC [PR116202]
>
> On Sun, Aug 4, 2024 at 1:47 PM <pan2...@intel.com> wrote:
> >
> > From: Pan Li <pan2...@intel.com>
> >
> > The .SAT_TRUNC matching can only perform the type has its mode
> > precision.
> >
> > g_12 = (long unsigned int) _2;
> > _13 = MIN_EXPR <g_12, 1>;
> > _3 = (_Bool) _13;
> >
> > The above pattern cannot be recog as .SAT_TRUNC (g_12) because the dest
> > only has 1 bit precision but QImode. Aka the type doesn't have the mode
> > precision. Thus, add the type_has_mode_precision_p for the dest to
> > avoid such case.
> >
> > The below tests are passed for this patch.
> > 1. The rv64gcv fully regression tests.
> > 2. The x86 bootstrap tests.
> > 3. The x86 fully regression tests.
>
> Isn't that now handled by the direct_internal_fn_supported_p check? That is,
> by the caller which needs to verify the matched operation is supported by
> the target?
>
> > PR target/116202
> >
> > gcc/ChangeLog:
> >
> > * match.pd: Add type_has_mode_precision_p for the dest type
> > of the .SAT_TRUNC matching.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.target/riscv/rvv/base/pr116202-run-1.c: New test.
> >
> > Signed-off-by: Pan Li <pan2...@intel.com>
> > ---
> > gcc/match.pd | 6 +++--
> > .../riscv/rvv/base/pr116202-run-1.c | 24 +++++++++++++++++++
> > 2 files changed, 28 insertions(+), 2 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
> >
> > diff --git a/gcc/match.pd b/gcc/match.pd
> > index c9c8478d286..dfa0bba3908 100644
> > --- a/gcc/match.pd
> > +++ b/gcc/match.pd
> > @@ -3283,7 +3283,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > wide_int trunc_max = wi::mask (otype_precision, false, itype_precision);
> > wide_int int_cst = wi::to_wide (@1, itype_precision);
> > }
> > - (if (otype_precision < itype_precision && wi::eq_p (trunc_max,
> > int_cst))))))
> > + (if (type_has_mode_precision_p (type) && otype_precision <
> > itype_precision
> > + && wi::eq_p (trunc_max, int_cst))))))
> >
> > /* Unsigned saturation truncate, case 2, sizeof (WT) > sizeof (NT).
> > SAT_U_TRUNC = (NT)(MIN_EXPR (X, 255)). */
> > @@ -3309,7 +3310,8 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > wide_int trunc_max = wi::mask (otype_precision, false, itype_precision);
> > wide_int int_cst = wi::to_wide (@1, itype_precision);
> > }
> > - (if (otype_precision < itype_precision && wi::eq_p (trunc_max,
> > int_cst))))))
> > + (if (type_has_mode_precision_p (type) && otype_precision <
> > itype_precision
> > + && wi::eq_p (trunc_max, int_cst))))))
> >
> > /* x > y && x != XXX_MIN --> x > y
> > x > y && x == XXX_MIN --> false . */
> > diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
> > b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
> > new file mode 100644
> > index 00000000000..d150f20b5d9
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/pr116202-run-1.c
> > @@ -0,0 +1,24 @@
> > +/* { dg-do run } */
> > +/* { dg-options "-O3 -march=rv64gcv_zvl256b -fdump-rtl-expand-details" } */
> > +
> > +int b[24];
> > +_Bool c[24];
> > +
> > +int main() {
> > + for (int f = 0; f < 4; ++f)
> > + b[f] = 6;
> > +
> > + for (int f = 0; f < 24; f += 4)
> > + c[f] = ({
> > + int g = ({
> > + unsigned long g = -b[f];
> > + 1 < g ? 1 : g;
> > + });
> > + g;
> > + });
> > +
> > + if (c[0] != 1)
> > + __builtin_abort ();
> > +}
> > +
> > +/* { dg-final { scan-rtl-dump-not ".SAT_TRUNC " "expand" } } */
> > --
> > 2.43.0
> >
