> 2023年7月12日 15:44,Richard Biener <richard.guent...@gmail.com> 写道: > > On Wed, Jul 12, 2023 at 5:20 AM YunQiang Su <yunqiang...@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
diff --git a/gcc/expmed.cc b/gcc/expmed.cc index fbd4ce2d42f..feee8c82f59 100644 --- a/gcc/expmed.cc +++ b/gcc/expmed.cc @@ -850,6 +861,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, 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 str_mode = int_mode_for_mode (GET_MODE (str_rtx)); scalar_int_mode imode; if (!op0_mode.exists (&imode) || imode != GET_MODE (op0)) { @@ -881,8 +893,14 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum, op0 = gen_lowpart (op0_mode.require (), op0); } - return store_integral_bit_field (op0, op0_mode, ibitsize, ibitnum, - bitregion_start, bitregion_end, + bool use_str_mode = false; + if (GET_MODE_CLASS(GET_MODE (str_rtx)) == MODE_INT + && GET_MODE_CLASS(GET_MODE (op0)) == MODE_INT + && known_gt (GET_MODE_SIZE(GET_MODE(op0)), GET_MODE_SIZE(GET_MODE(str_rtx)))) + use_str_mode = true; + return store_integral_bit_field (op0, + use_str_mode ? str_mode : op0_mode, + ibitsize, ibitnum, bitregion_start, bitregion_end, fieldmode, value, reverse, fallback_p); } > reason. I also wonder why we don't just check GET_MODE_CLASS > (GET_MODE (op0)) == MODE_CLASS_INT ... > In fact I have no idea. Maybe there are some other tricky cases. >> scalar_int_mode imode; >> if (!op0_mode.exists (&imode) || imode != GET_MODE (op0)) >> { >> -- >> 2.30.2