Re: [patch] fold-const.c: Don't transform X & C into (X >> C') & 1in fold_binary.

[Jeffrey A Law <law at redhat dot com>]

On Thu, 2005-04-21 at 07:42 +0100, Richard Sandiford wrote:
> Kazu Hirata <kazu@cs.umass.edu> writes:
> >> If your argument holds true, I'd be glad to see both the fold code
> >> and the do_jump code removed.  (At least, assuming I've understood
> >> correctly, and that this:
> >> 
> >> > AFAICT, it does not matter whether we do this transformation or not as
> >> > far as conditional branches are concerned.  Combine canonicalizes a
> >> > single bit test into zero_extract regardless of whether we have
> >> >
> >> >   a & 4
> >> >
> >> > or
> >> >
> >> >   (a >> 2) & 1
> >> 
> >> means that, even without the jump code, we will continue to avoid the
> >> shift on MIPS if the user writes "(X >> 2) & 1" themselves.)
> >
> > Oh, I should have been a bit more careful here.  I meant that if we
> > are setting cc0 or a MODE_CC register, combine canonicalizes a single
> > bit test into zero_extract (regardless of whether we have a & 4 or
> > (a >> 2) & 1.  If the MIPS port has no such insn, I suspect with my
> > patch, (a >> 2) & 1 would stay as is.
> 
> Ah!  That would certainly be a problem.  MIPS doesn't use MODE_CC for
> integer comparisons.  The branches compare word-mode registers against
> either another register or zero.
> 
> > If you like, I'd be happy to teach fold_binary to transform
> > (a >> 2) & 1 != 0 back to a & 4 != 0 for you
> 
> Sounds good.  FWIW, this is quite an important issue for MIPS: it led
> to a significant loss of performance in a well-known embedded benchmark
> (one that I can't name due to silly licence restrictions).
> 
> In current sources, both:
> 
>     void f1 (int x) { if (x & 4) bar (); }
> and:
>     void f2 (int x) { if ((x >> 2) & 1) bar (); }
> 
> will be implemented using an "and" with 4 followed by a branch on zero.
> We definitely want to keep this behaviour.  (I think the do_jump code
> is effectively providing the canonical form you're talking about,
> at least as far as branches go.)
You know, it might be wise to put a test for this in the testsuite if
we don't have one already.  

jeff