> -----Original Message-----
> From: Richard Sandiford <richard.sandif...@arm.com>
> Sent: Wednesday, October 25, 2023 4:40 PM
> To: HAO CHEN GUI <guih...@linux.ibm.com>
> Cc: Jiang, Haochen <haochen.ji...@intel.com>; gcc-patches <gcc-
> patc...@gcc.gnu.org>
> Subject: Re: [PATCH-1v4, expand] Enable vector mode for compare_by_pieces
> [PR111449]
>
> HAO CHEN GUI <guih...@linux.ibm.com> writes:
> > Hi Haochen,
> > The regression cases are caused by "targetm.scalar_mode_supported_p"
> > I added for scalar mode checking. XImode, OImode and TImode (with
> > -m32) are not enabled in ix86_scalar_mode_supported_p. So they're
> > excluded from by pieces operations on i386.
> >
> > The original code doesn't do a check for scalar modes. I think it
> > might be incorrect as not all scalar modes support move and compare optabs.
> > (e.g.
> > TImode with -m32 on rs6000).
> >
> > I drafted a new patch to manually check optabs for scalar mode. Now
> > both vector and scalar modes are checked for optabs.
> >
> > I did a simple test. All former regression cases are back. Could you
> > help do a full regression test? I am worry about the coverage of my CI
> > system.
Thanks for that. I am running the regression test now.
Thx,
Haochen
>
> Thanks for the quick fix. The patch LGTM FWIW. Just a small suggestion for
> the function name:
>
> >
> > Thanks
> > Gui Haochen
> >
> > patch.diff
> > diff --git a/gcc/expr.cc b/gcc/expr.cc index 7aac575eff8..2af9fcbed18
> > 100644
> > --- a/gcc/expr.cc
> > +++ b/gcc/expr.cc
> > @@ -1000,18 +1000,21 @@ can_use_qi_vectors (by_pieces_operation op)
> > /* Return true if optabs exists for the mode and certain by pieces
> > operations. */
> > static bool
> > -qi_vector_mode_supported_p (fixed_size_mode mode,
> by_pieces_operation
> > op)
> > +mode_supported_p (fixed_size_mode mode, by_pieces_operation op)
>
> Might be worth calling this something more specific, such as
> by_pieces_mode_supported_p.
>
> Otherwise the patch is OK for trunk if it passes the x86 testing.
>
> Thanks,
> Richard
>
> > {
> > + if (optab_handler (mov_optab, mode) == CODE_FOR_nothing)
> > + return false;
> > +
> > if ((op == SET_BY_PIECES || op == CLEAR_BY_PIECES)
> > - && optab_handler (vec_duplicate_optab, mode) != CODE_FOR_nothing)
> > - return true;
> > + && VECTOR_MODE_P (mode)
> > + && optab_handler (vec_duplicate_optab, mode) ==
> CODE_FOR_nothing)
> > + return false;
> >
> > if (op == COMPARE_BY_PIECES
> > - && optab_handler (mov_optab, mode) != CODE_FOR_nothing
> > - && can_compare_p (EQ, mode, ccp_jump))
> > - return true;
> > + && !can_compare_p (EQ, mode, ccp_jump))
> > + return false;
> >
> > - return false;
> > + return true;
> > }
> >
> > /* Return the widest mode that can be used to perform part of an @@
> > -1035,7 +1038,7 @@ widest_fixed_size_mode_for_size (unsigned int size,
> by_pieces_operation op)
> > {
> > if (GET_MODE_SIZE (candidate) >= size)
> > break;
> > - if (qi_vector_mode_supported_p (candidate, op))
> > + if (mode_supported_p (candidate, op))
> > result = candidate;
> > }
> >
> > @@ -1049,7 +1052,7 @@ widest_fixed_size_mode_for_size (unsigned int
> size, by_pieces_operation op)
> > {
> > mode = tmode.require ();
> > if (GET_MODE_SIZE (mode) < size
> > - && targetm.scalar_mode_supported_p (mode))
> > + && mode_supported_p (mode, op))
> > result = mode;
> > }
> >
> > @@ -1454,7 +1457,7 @@
> op_by_pieces_d::smallest_fixed_size_mode_for_size (unsigned int size)
> > break;
> >
> > if (GET_MODE_SIZE (candidate) >= size
> > - && qi_vector_mode_supported_p (candidate, m_op))
> > + && mode_supported_p (candidate, m_op))
> > return candidate;
> > }
> > }
