On Fri, Dec 09, 2016 at 04:23:44PM +0100, Dominik Vogt wrote:
> 0001-*
> 
>   Deal with mode expanding zero_extracts in change_zero_ext.  The
>   patch looks good to me, but not sure whether endianness is
>   handled properly.  Is the second argument of gen_rtx_SUBREG
>   correct?

> >From 600ed3dadd5bc2568ab53be8466686abaf27ff3f Mon Sep 17 00:00:00 2001
> From: Dominik Vogt <v...@linux.vnet.ibm.com>
> Date: Fri, 9 Dec 2016 02:48:30 +0100
> Subject: [PATCH 1/2] combine: Handle mode expanding zero_extracts in
>  change_zero_ext.
> 
> Example:
> 
>   (zero_extract:DI (reg:SI)
>                    (const_int 24)
>                    (const_int 0))
> 
> -->
> 
>   (and:DI (subreg:DI (lshiftrt:SI (reg:SI) (const_int 8))
>                      0)
>           (const_int 16777215))
> ---
>  gcc/combine.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/combine.c b/gcc/combine.c
> index b429453..e14a08f 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -11237,18 +11237,24 @@ change_zero_ext (rtx pat)
>        if (GET_CODE (x) == ZERO_EXTRACT
>         && CONST_INT_P (XEXP (x, 1))
>         && CONST_INT_P (XEXP (x, 2))
> -       && GET_MODE (XEXP (x, 0)) == mode)
> +       && (GET_MODE (XEXP (x, 0)) == mode
> +           || GET_MODE_PRECISION (GET_MODE (XEXP (x, 0)))
> +              < GET_MODE_PRECISION (mode)))

If you make this <= you can collapse the two cases into one.

>       {
> +       machine_mode inner_mode = GET_MODE (XEXP (x, 0));
> +
>         size = INTVAL (XEXP (x, 1));
>  
>         int start = INTVAL (XEXP (x, 2));
>         if (BITS_BIG_ENDIAN)
> -         start = GET_MODE_PRECISION (mode) - size - start;
> +         start = GET_MODE_PRECISION (inner_mode) - size - start;
>  
>         if (start)
> -         x = gen_rtx_LSHIFTRT (mode, XEXP (x, 0), GEN_INT (start));
> +         x = gen_rtx_LSHIFTRT (inner_mode, XEXP (x, 0), GEN_INT (start));
>         else
>           x = XEXP (x, 0);
> +       if (mode != inner_mode)
> +         x = gen_rtx_SUBREG (mode, x, 0);

gen_lowpart_SUBREG instead?  It's easier to read, and code a little
further down does the same thing.

Okay for trunk with those changes (did you bootstrap+regcheck this
already?)  Thanks,


Segher

Reply via email to