[PATCH 2/2] combine: Don't turn (mult (extend x) 2^n) into extract

Segher Boessenkool segher@kernel.crashing.org
Mon Oct 26 11:51:48 GMT 2020


On Mon, Oct 26, 2020 at 11:06:22AM +0000, Alex Coplan wrote:
> Well, only the low 32 bits of the subreg are valid. But because those
> low 32 bits are shifted left 2 times, the low 34 bits of the ashift are
> valid: the bottom 2 bits of the ashift are zeros, and the 32 bits above
> those are from the inner SImode reg (with the upper 62 bits being
> undefined).

Ugh.  Yes, I think you are right.  One more reason why we should only
use *explicit* sign/zero extends, none of this confusing subreg
business :-(

> > > diff --git a/gcc/combine.c b/gcc/combine.c
> > > index c88382efbd3..fe8eff2b464 100644
> > > --- a/gcc/combine.c
> > > +++ b/gcc/combine.c
> > > @@ -7419,8 +7419,8 @@ expand_compound_operation (rtx x)
> > >      }
> > >  
> > >    /* If we reach here, we want to return a pair of shifts.  The inner
> > > -     shift is a left shift of BITSIZE - POS - LEN bits.  The outer
> > > -     shift is a right shift of BITSIZE - LEN bits.  It is arithmetic or
> > > +     shift is a left shift of MODEWIDTH - POS - LEN bits.  The outer
> > > +     shift is a right shift of MODEWIDTH - LEN bits.  It is arithmetic or
> > >       logical depending on the value of UNSIGNEDP.
> > >  
> > >       If this was a ZERO_EXTEND or ZERO_EXTRACT, this pair of shifts will be
> > 
> > MODEWIDTH isn't defined here yet, it is initialised just below to
> > MODE_PRECISION (mode).
> 
> Yes, but bitsize isn't defined at all in this function AFAICT. Are
> comments not permitted to refer to variables defined immediately beneath
> them?

Of course you can -- comments are free form text after all -- but as
written it suggest there already is an initialised variable "modewidth".

Just move the initialisation to above this comment?


Segher


More information about the Gcc-patches mailing list