Re: [RFC] Store_bit_field_1: Use mode of SUBREG instead of REG

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Jul 12, 2023 at 5:20 AM YunQiang Su <yunqiang.su@cipunited.com> wrote:
>
> PR #104914
>
> When work with
>   int val;
>   ((unsigned char*)&val)[0] = *buf;
> The RTX mode is obtained from REG instead of SUBREG,
> which make D<INS> is used instead of <INS>.
> Thus something wrong happens on sign-extend default architectures,
> like MIPS64.
>
> gcc/ChangeLog:
>         PR: 104914.
>         * expmed.cc(store_bit_field_1): Get mode from original
>         str_rtx instead of op0.
> ---
>  gcc/expmed.cc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
> index fbd4ce2d42f..37f90912122 100644
> --- a/gcc/expmed.cc
> +++ b/gcc/expmed.cc
> @@ -849,7 +849,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
>       if we aren't.  This must come after the entire register case above,
>       since that case is valid for any mode.  The following cases are only
>       valid for integral modes.  */
> -  opt_scalar_int_mode op0_mode = int_mode_for_mode (GET_MODE (op0));
> +  opt_scalar_int_mode op0_mode = int_mode_for_mode (GET_MODE (str_rtx));

I don't think this is correct - op0_mode is used to store into op0, and we are
just requiring that it is an integer mode and equal to the original
mode.  I suppose
your patch makes us go to the fallback code instead, but it's surely
for the wrong
reason.  I also wonder why we don't just check GET_MODE_CLASS
(GET_MODE (op0)) == MODE_CLASS_INT ...

>    scalar_int_mode imode;
>    if (!op0_mode.exists (&imode) || imode != GET_MODE (op0))
>      {
> --
> 2.30.2
>