Re: [PATCH][committed]middle-end intialize regnum in store_bit_field_1

[Richard Biener <rguenther@suse.de>]

On Tue, 30 Aug 2022, Richard Sandiford wrote:

> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> > On Tue, 30 Aug 2022, Tamar Christina wrote:
> >
> >> Hi All,
> >> 
> >> This initializes regnum to 0 for when undefined_p.
> >> 0 is the right default as it's supposed to get the lowpart
> >> when undefined.
> >> 
> >> Bootstrapped Regtested on aarch64-none-linux-gnu and no issues.
> >> 
> >> Ok for master?
> >
> > OK.
> 
> I'm not sure this is the right fix.  We were still supposed to use the
> correct byte offset in the undefined case.
> 
> Is the attached OK?  Tested on aarch64-linux-gnu.

OK.

Thanks,
Richard.

> Thanks,
> Richard
> 
> ----------------------
> 
> store_bit_field_1 tries to convert a field assignment into a subreg
> assignment.  Normally it must check that the field occupies a full
> word (or more specifically, a full REGMODE_NATURAL_SIZE chunk),
> so that writing to the subreg doesn't clobber any other fields.
> But it can skip that check if the structure is known to be in
> an undefined state.
> 
> The idea was that, in the undefined case, we could rely on
> simplify_gen_subreg to do the check for a valid subreg, rather
> than having to repeat the required endianness logic in the caller.
> 
> Before the addition of the undefined case, the code could use
> regnum * regsize to get the byte offset, where regnum came from
> checking that the start was word-aligned.  In the undefined case
> we need to calculate the byte offset explicitly.
> 
> gcc/
> 	* expmed.cc (store_bit_field_1): Fix byte offset calculation
> 	for undefined structures.
> ---
>  gcc/expmed.cc | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/expmed.cc b/gcc/expmed.cc
> index 8d7418be418..6c02c3bb850 100644
> --- a/gcc/expmed.cc
> +++ b/gcc/expmed.cc
> @@ -794,7 +794,7 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
>  	 words or to cope with mode punning between equal-sized modes.
>  	 In the latter case, use subreg on the rhs side, not lhs.  */
>        rtx sub;
> -      HOST_WIDE_INT regnum;
> +      poly_uint64 bytenum;
>        poly_uint64 regsize = REGMODE_NATURAL_SIZE (GET_MODE (op0));
>        if (known_eq (bitnum, 0U)
>  	  && known_eq (bitsize, GET_MODE_BITSIZE (GET_MODE (op0))))
> @@ -808,13 +808,13 @@ store_bit_field_1 (rtx str_rtx, poly_uint64 bitsize, poly_uint64 bitnum,
>  	      return true;
>  	    }
>  	}
> -      else if (((constant_multiple_p (bitnum, regsize * BITS_PER_UNIT, &regnum)
> -		 && multiple_p (bitsize, regsize * BITS_PER_UNIT))
> -		|| undefined_p)
> +      else if (multiple_p (bitnum, BITS_PER_UNIT, &bytenum)
> +	       && (undefined_p
> +		   || (multiple_p (bitnum, regsize * BITS_PER_UNIT)
> +		       && multiple_p (bitsize, regsize * BITS_PER_UNIT)))
>  	       && known_ge (GET_MODE_BITSIZE (GET_MODE (op0)), bitsize))
>  	{
> -	  sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0),
> -				     regnum * regsize);
> +	  sub = simplify_gen_subreg (fieldmode, op0, GET_MODE (op0), bytenum);
>  	  if (sub)
>  	    {
>  	      if (reverse)
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)