> 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


Reply via email to