This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Bernd Schmidt <bschmidt at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 12 Feb 2016 14:45:40 +0100 (CET)
- Subject: Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion
- Authentication-results: sourceware.org; auth=none
- References: <alpine dot LSU dot 2 dot 11 dot 1602121119020 dot 31122 at t29 dot fhfr dot qr> <20160212103203 dot GM3017 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1602121145370 dot 31122 at t29 dot fhfr dot qr> <56BDCCCF dot 2030601 at redhat dot com> <20160212122438 dot GP3017 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1602121346100 dot 31122 at t29 dot fhfr dot qr> <20160212125729 dot GQ3017 at tucnak dot redhat dot com>
On Fri, 12 Feb 2016, Jakub Jelinek wrote:
> On Fri, Feb 12, 2016 at 01:48:36PM +0100, Richard Biener wrote:
> > 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 case I'm worried about is if xop1 is a constant in narrower
> mode (let's say QImode), e.g. -15, unsignedp is 1, and xmode1 is
> wider. Then previously we'd zero extend it, thus get
> 0xf1, but with your patch we'll end up with -15 instead,
> because convert_modes will be called e.g. with (SImode, VOIDmode, xop1, 1)
> instead of (SImode, QImode, xop1, 1).
> Dunno if it is just hypothetical or real.
But we don't know which mode xop1 was expanded with here. The only
way to know would be passing down its original type (or its mode).
That's the general issue with our modeless CONST_INTs.
So yes, that case is sth to worry about (for all operations that
can arrive in expand_binop_directly which can have different mode
operands).
Also note that unsignedp doesn't apply to op1 for shifts, only to op0.
So eventually we shouldn't (ab-)use expand_binop for expanding shifts
at all...
Richard.