Re: [PATCH, rtl-optimization]: Fix PR37997: ICE shifting byte to the right with constant > 7FFFFFFF

[Michael Matz <matz at suse dot de>]

Hi,

On Thu, 10 Sep 2009, Uros Bizjak wrote:

> > > convert_modes (enum machine_mode mode, enum machine_mode oldmode, rtx x,
> > > int
> > >
> > > Converting QImode to QImode should be a no-op. ïI think shifts should be
> > > handled specially in expand_binop_directly then.
> > >
> > >      
> > I agree. This change made gcc to run slower for some questionable usage.
> >    
> 
> What questionable usage?
> 
> The patch fixes the problem, exposed by unfortunate testcase. The 
> problem was that huge CONST_INT didn't satisfy QImode constraints,

And _that_ IMO is the problem, not that convert_modes did something wrong 
with that.  Garbage in, garbage out.  You worked around that by accepting 
a bit more garbage in convert_modes while still retaining it's output 
contract (namely that the output fits into the requested mode).  It would 
have been better to fix it at the place that generates the garbage input 
(i.e. where the too large constant for QImode is generated, or at least 
before it is passed to convert_modes).  That doesn't necessarily mean the 
frontend itself.

I.e. if I'd put such an assert at the beginning of convert_modes:

  if (const_int_p (x))
    gcc_assert (int_fits_mode (x, oldmode, unsignedp));

(with obviously defined const_int_p/int_fits_mode) I would expect this to 
never trigger and declare all triggers incorrect usage of convert_modes.

I think that is what Eric was getting at, and I agree.

If it never triggers, then indeed convert_modes (mode, mode, x, u) is a 
no-op, as it should be.


Ciao,
Michael.