> -----Original Message-----
> From: Jiang, Haochen
> Sent: Wednesday, October 25, 2023 4:47 PM
> To: Richard Sandiford <richard.sandif...@arm.com>; HAO CHEN GUI
> <guih...@linux.ibm.com>
> Cc: gcc-patches <gcc-patches@gcc.gnu.org>
> Subject: RE: [PATCH-1v4, expand] Enable vector mode for compare_by_pieces
> [PR111449]
>
> > -----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.
The patch works. Thanks a lot!
Sorry for the delay since my CI accidentally crashed.
Thx,
Haochen
>
> 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;
> > > }
> > > }
