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: Jakub Jelinek <jakub at redhat dot com>
- To: Richard Biener <rguenther at suse dot de>
- Cc: Bernd Schmidt <bschmidt at redhat dot com>, gcc-patches at gcc dot gnu dot org
- Date: Fri, 12 Feb 2016 14:49:50 +0100
- 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> <alpine dot LSU dot 2 dot 11 dot 1602121440390 dot 31122 at t29 dot fhfr dot qr>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Fri, Feb 12, 2016 at 02:45:40PM +0100, Richard Biener wrote:
> > 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).
Well, most binary ops have the requirement that both modes are the same,
which is why most of the expansion APIs for them pass around just a single
mode, not two modes (or 3, if even the result could have different mode).
And in that case we better have expanded the CONST_INTs with the right mode
already. Just shifts are special.
> 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...
Perhaps; but please look at the latest patch I've posted, which IMHO does
what your patch does for shift/rorate xop1 only, and keeps doing what it
used to do otherwise.
Jakub