This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH v3] Do not simplify "(and (reg) (const bit))" to if_then_else.


On Mon, Dec 05, 2016 at 04:00:25AM -0600, Segher Boessenkool wrote:
> On Mon, Dec 05, 2016 at 10:22:13AM +0100, Dominik Vogt wrote:
> > On Sat, Dec 03, 2016 at 07:19:13PM -0600, Segher Boessenkool wrote:
> > > [ I did not see this patch before, sorry. ]
> > > 
> > > This causes the second half of PR78638.
> > > 
> > > On Thu, Dec 01, 2016 at 04:30:08PM +0100, Dominik Vogt wrote:
> > > > --- a/gcc/combine.c
> > > > +++ b/gcc/combine.c
> > > > @@ -5600,6 +5600,18 @@ combine_simplify_rtx (rtx x, machine_mode op0_mode, int in_dest,
> > > >  		     && OBJECT_P (SUBREG_REG (XEXP (x, 0)))))))
> > > >      {
> > > >        rtx cond, true_rtx, false_rtx;
> > > > +      unsigned HOST_WIDE_INT nz;
> > > > +
> > > > +      /* If the operation is an AND wrapped in a SIGN_EXTEND or ZERO_EXTEND with
> > > > +	 either operand being just a constant single bit value, do nothing since
> > > > +	 IF_THEN_ELSE is likely to increase the expression's complexity.  */
> > > > +      if (HWI_COMPUTABLE_MODE_P (mode)
> > > > +	  && pow2p_hwi (nz = nonzero_bits (x, mode))
> > > > +	  && ! ((code == SIGN_EXTEND || code == ZERO_EXTEND)
> > > > +		&& GET_CODE (XEXP (x, 0)) == AND
> > > > +		&& CONST_INT_P (XEXP (XEXP (x, 0), 0))
> > > > +		&& UINTVAL (XEXP (XEXP (x, 0), 0)) == nz))
> > > > +	      return x;
> > > 
> > > The code does not match the comment: the "!" should not be there.  How
> > > did it fix anything on s390 *with* that "!"?  That does not make much
> > > sense.
> > 
> > Sorry for breaking this.  With the constant changes in the
> > patterns this is supposed to fix it seems I've lost track of the
> > status quo.  I'll check what went wrong with the patch; in the
> > mean time Andreas will revert this, or if it's urgent, feel free
> > to do that yourself.
> 
> I have tested that removing that ! cures all regressions.  I do not
> know if it still fixes what this patch intended to fix, of course.

S390[x] has these complicated patterns for the risbg instruction,
and there's some ongoing work on patterns for related instructions
(rosbg, rxsbg) which needed the patch discussed here - at least at
some point in time.  But the risbg patterns are breaking all over
the place because they are so sensitive to changes in combine.c
(and possibly other passes), and any change fixing the old
patterns may affect the new ones.  In other words:  At the moment
I have no clue whether the discussed patch is still good for
anythin on s390[x].

If there was a consensus that the patch discussed here, with the
"!" fix is useful in any case, that would simplify my current
work, but

1) I've done no testing with it (only with the broken version of
   the patch),
2) it may be just a chunk of dead code.

Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]