[PATCH] Fix PR69771, bogus CONST_INT during shift expansion

[Richard Biener]

On Fri, 12 Feb 2016, Richard Biener wrote:

> On Fri, 12 Feb 2016, Jakub Jelinek wrote:
> 
> > On Fri, Feb 12, 2016 at 01:15:11PM +0100, Bernd Schmidt wrote:
> > > >Ah, indeed looks like a dup.  Let's go with your version which had
> > > >feedback from Bernd already.  Might want to add my testcase (w/o the
> > > >runtime outcome test).
> > > 
> > > Actually, Richard I was just looking at Jakub's second patch and I think
> > > yours is better - on the first round of review I didn't notice that the
> > > convert_modes code is there and is documented to deal with the CONST_INT
> > > problem. If it completed testing I think you should commit it.
> > 
> > That patch doesn't look right to me.  The code is there not just for shifts,
> > but also for non-shifts, and for non-shifts, we know the arguments are in
> > mode, we also know unsignedp, so if needed convert_modes can perform
> > zero or sign extension.  IMHO it is just shifts/rotates that are
> > problematical, because what the second operand mode is and whether it is unsigned
> > or signed is just less well defined, and then various backends have various
> > requirements on it.  Also, on some target for shift/rotate xmode1 might be
> > already equal to mode, and in that case convert_modes would not be called,
> > but still the CONST_INT might be originally from yet another mode and we'd
> > still ICE.
> 
> But my patch should deal with all this.
> 
> -  mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode;
> -  if (xmode1 != VOIDmode && xmode1 != mode1)
> +  mode1 = GET_MODE (xop1);
> +  if (xmode1 != mode1)
>      {
>        xop1 = convert_modes (xmode1, mode1, xop1, unsignedp);
>        mode1 = xmode1;
> 
> so if xop1 is not VOIDmode and already of xmode1 we won't do anything.
> But if it is VOIDmode (and xmode1 is not VOIDmode) we'll always
> do convert_modes.
> 
> The only thing that can (hopefully not) happen is that xmode1
> is VOIDmode but mode1 is not (maybe removing the xmode1 != VOIDmode
> check is a bit too optimistic here).  Not sure if there can be
> valid patterns requesting a VOIDmode op ... (and not only accept
> CONST_INTs).

Oh, bootstrap & testing went fine on x86_64-unknown-linux-gnu.

If we can agree on the patch then I'll pick up the testcase from
your patch (and adjust mine).

Richard.