Re: [PATCH] Small optimization for emit_case_bit_tests (PR tree-optimization/63464)

[Richard Biener <rguenther at suse dot de>]

On Fri, 10 Oct 2014, Jakub Jelinek wrote:

> On Fri, Oct 10, 2014 at 12:55:21PM +0200, Richard Biener wrote:
> > On Fri, 10 Oct 2014, Jakub Jelinek wrote:
> > > This patch adds a small optimization to emit_case_bit_tests,
> > > instead of emitting (for high, low, mask all constants)
> > > (x - low) <= (high - low) && ((1 << (x - low)) & mask)
> > > if high is smaller than BITS_PER_WORD and low > 0 we can emit
> > > x <= high && ((1 << x) & (mask << low))
> > > and avoid subtraction.  Do this only if mask << low isn't more costly than
> > > mask.
> > > 
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > But isn't this a general optimization?
> 
> The && always has to mean separate basic blocks, as otherwise it is
> undefined behavior.  I'd think this optimization would be too specialized
> for a general optimization, and unsure in which pass it would be desirable.

Yeah, I also didn't come up with a good place to do this in general.
ifcombine maybe, which already does some pattern matching on
if (x) if (y) ...

If I understand correctly we have

 if ((x - low) <= (high - low))
  {
    if (((1 << (x - low)) & mask))
      {
     ...

which we'd like to transform to

 if (x <= high)
   {
     if ((1 << x) & (mask << low))
       {
...

ifcombine can certainly detect the pattern, but obviously the
result wouldn't have the ifs combined in any way.  And you'd
need to check whether _all_ code that depends on both tests
is ok with the transformed checks (not sure if that is always
the case).

> > Also testing for RTX costs this early sounds bogus.
> 
> Well, the bit test optimization is already decided based on other rtx costs.

Oh, I see.

I suppose the patch is fine then.

Thanks,
Richard.