[PATCH] combine: Omit redundant AND in change_zero_ext.

[PATCH] combine: Omit redundant AND in change_zero_ext.

[Dominik Vogt]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

This is another micro-optimisation in change_zero_ext.  If an

  (and (lshiftrt ... (N)) (M))

generated by change_zero_ext is equivalent to just

  (lshiftrt ... (N))

(because the AND constant selects the N rightmost bits of the
result), strip off the AND.

_But_ I'm still not completely convinced whether this is a good
idea.  It may become necessary to add md patterns to deal with
just the LSHIFTRT.  On the other hand it saves the need for
another special case in change_zero_ext, and a less obvious, very
specific risbg pattern on s390 

--

Bootstrapped and regression tested on s390x and s390.  (Targets
with risbg-like instructions (Power, others?) may need some
tuning.)

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

[-- Attachment #2: 0001-ChangeLog --]
[-- Type: text/plain, Size: 99 bytes --]

gcc/ChangeLog-change_zero_ext-2

	* combine.c (change_zero_ext): Skip generation of redundant AND.

[-- Attachment #3: 0001-combine-Omit-redundant-AND-in-change_zero_ext.patch --]
[-- Type: text/plain, Size: 1043 bytes --]

From bbd2cfc122c74d1e50894222a7998915848b5ec6 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <vogt@linux.vnet.ibm.com>
Date: Tue, 13 Dec 2016 12:37:08 +0100
Subject: [PATCH] combine: Omit redundant AND in change_zero_ext.

In case (and (lshiftrt)) is equivalent to just (lshiftrt).
---
 gcc/combine.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gcc/combine.c b/gcc/combine.c
index 19851a2..5ebf31c 100644
--- a/gcc/combine.c
+++ b/gcc/combine.c
@@ -11280,8 +11280,13 @@ change_zero_ext (rtx pat)
       else
 	continue;
 
-      wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
-      x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
+      if (!(GET_CODE (x) == LSHIFTRT
+	    && CONST_INT_P (XEXP (x, 1))
+	    && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
+	{
+	  wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
+	  x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
+	}
 
       SUBST (**iter, x);
       changed = true;
-- 
2.3.0

Re: [PATCH] combine: Omit redundant AND in change_zero_ext.

[Segher Boessenkool]

On Wed, Dec 14, 2016 at 11:01:47AM +0100, Dominik Vogt wrote:
> This is another micro-optimisation in change_zero_ext.  If an
> 
>   (and (lshiftrt ... (N)) (M))
> 
> generated by change_zero_ext is equivalent to just
> 
>   (lshiftrt ... (N))
> 
> (because the AND constant selects the N rightmost bits of the
> result), strip off the AND.
> 
> _But_ I'm still not completely convinced whether this is a good
> idea.  It may become necessary to add md patterns to deal with
> just the LSHIFTRT.  On the other hand it saves the need for
> another special case in change_zero_ext, and a less obvious, very
> specific risbg pattern on s390 

For PowerPC we should already have all such patterns with a "bare" shift
(they can be created in other ways, too).

> Bootstrapped and regression tested on s390x and s390.  (Targets
> with risbg-like instructions (Power, others?) may need some
> tuning.)

But, it is also possible I missed some.  So please wait until I have
tested it.


> diff --git a/gcc/combine.c b/gcc/combine.c
> index 19851a2..5ebf31c 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -11280,8 +11280,13 @@ change_zero_ext (rtx pat)
>        else
>  	continue;
>  
> -      wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> -      x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> +      if (!(GET_CODE (x) == LSHIFTRT
> +	    && CONST_INT_P (XEXP (x, 1))
> +	    && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
> +	{
> +	  wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> +	  x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> +	}

One could argue that this should have been an lshiftrt in the first place
then, not a zero_ext*.  Hrm.


Segher

Re: [PATCH] combine: Omit redundant AND in change_zero_ext.

[Dominik Vogt]

On Wed, Dec 14, 2016 at 01:32:48PM -0600, Segher Boessenkool wrote:
> On Wed, Dec 14, 2016 at 11:01:47AM +0100, Dominik Vogt wrote:
> > This is another micro-optimisation in change_zero_ext.  If an
> > 
> >   (and (lshiftrt ... (N)) (M))
> > 
> > generated by change_zero_ext is equivalent to just
> > 
> >   (lshiftrt ... (N))
> > 
> > (because the AND constant selects the N rightmost bits of the
> > result), strip off the AND.
> > 
> > _But_ I'm still not completely convinced whether this is a good
> > idea.  It may become necessary to add md patterns to deal with
> > just the LSHIFTRT.  On the other hand it saves the need for
> > another special case in change_zero_ext, and a less obvious, very
> > specific risbg pattern on s390 
> 
> For PowerPC we should already have all such patterns with a "bare" shift
> (they can be created in other ways, too).
> 
> > Bootstrapped and regression tested on s390x and s390.  (Targets
> > with risbg-like instructions (Power, others?) may need some
> > tuning.)
> 
> But, it is also possible I missed some.  So please wait until I have
> tested it.
> 
> 
> > diff --git a/gcc/combine.c b/gcc/combine.c
> > index 19851a2..5ebf31c 100644
> > --- a/gcc/combine.c
> > +++ b/gcc/combine.c
> > @@ -11280,8 +11280,13 @@ change_zero_ext (rtx pat)
> >        else
> >  	continue;
> >  
> > -      wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> > -      x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> > +      if (!(GET_CODE (x) == LSHIFTRT
> > +	    && CONST_INT_P (XEXP (x, 1))
> > +	    && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
> > +	{
> > +	  wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> > +	  x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> > +	}
> 
> One could argue that this should have been an lshiftrt in the first place
> then, not a zero_ext*.  Hrm.

This one

  void g2(ui64 *pl, i32 seed)
  {
    seed = 69607 * seed;
    pl[0] = (seed >> 8) & 0xffffff;
  }

generates

  (zero_extract:DI (reg:SI 75 [ seed ])
    (const_int 24 [0x18])
    (const_int 0 [0]))

on s390x.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany

Re: [PATCH] combine: Omit redundant AND in change_zero_ext.

[Segher Boessenkool]

On Thu, Dec 15, 2016 at 09:55:52AM +0100, Dominik Vogt wrote:
> > > Bootstrapped and regression tested on s390x and s390.  (Targets
> > > with risbg-like instructions (Power, others?) may need some
> > > tuning.)
> > 
> > But, it is also possible I missed some.  So please wait until I have
> > tested it.
> > 
> > > diff --git a/gcc/combine.c b/gcc/combine.c
> > > index 19851a2..5ebf31c 100644
> > > --- a/gcc/combine.c
> > > +++ b/gcc/combine.c
> > > @@ -11280,8 +11280,13 @@ change_zero_ext (rtx pat)
> > >        else
> > >  	continue;
> > >  
> > > -      wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> > > -      x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> > > +      if (!(GET_CODE (x) == LSHIFTRT
> > > +	    && CONST_INT_P (XEXP (x, 1))
> > > +	    && size + INTVAL (XEXP (x, 1)) == GET_MODE_PRECISION (mode)))
> > > +	{
> > > +	  wide_int mask = wi::mask (size, false, GET_MODE_PRECISION (mode));
> > > +	  x = gen_rtx_AND (mode, x, immed_wide_int_const (mask, mode));
> > > +	}
> > 
> > One could argue that this should have been an lshiftrt in the first place
> > then, not a zero_ext*.  Hrm.
> 
> This one
> 
>   void g2(ui64 *pl, i32 seed)
>   {
>     seed = 69607 * seed;
>     pl[0] = (seed >> 8) & 0xffffff;
>   }
> 
> generates
> 
>   (zero_extract:DI (reg:SI 75 [ seed ])
>     (const_int 24 [0x18])
>     (const_int 0 [0]))
> 
> on s390x.

Ah, right, it changes mode as well.  I see.

Tested on powerpc64-linux {-m32,-m64}, no new failures.  The patch is
okay for trunk.  Thanks!


Segher

Re: [PATCH] combine: Omit redundant AND in change_zero_ext.

[Andreas Krebbel]

On Wed, Dec 14, 2016 at 11:01:47AM +0100, Dominik Vogt wrote:
> gcc/ChangeLog-change_zero_ext-2
> 
> 	* combine.c (change_zero_ext): Skip generation of redundant AND.

Applied. Thanks!

-Andreas-