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 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;
> }
> }
