Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Tue, Feb 16, 2016 at 10:05:55AM +0100, Richard Biener wrote: > On Tue, 16 Feb 2016, Jakub Jelinek wrote: > > > On Mon, Feb 15, 2016 at 08:43:22PM +0100, Richard Biener wrote: > > > On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek > > > wrote: > > > >On Mon, Feb 15,

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Tue, 16 Feb 2016, Jakub Jelinek wrote: > On Mon, Feb 15, 2016 at 08:43:22PM +0100, Richard Biener wrote: > > On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek > > wrote: > > >On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote: > > >> We could also

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Mon, Feb 15, 2016 at 08:43:22PM +0100, Richard Biener wrote: > On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek > wrote: > >On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote: > >> We could also force_reg those at expansion or apply >

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek wrote: >On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote: >> We could also force_reg those at expansion or apply >SHIFT_COUNT_TRUNCATED to those invalid constants there. > >Sure, but for force_reg we'd still

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote: > We could also force_reg those at expansion or apply SHIFT_COUNT_TRUNCATED to > those invalid constants there. Sure, but for force_reg we'd still need the gen_int_mode anyway. As for SHIFT_COUNT_TRUNCATED, it should have been

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On February 15, 2016 4:34:38 PM GMT+01:00, Jakub Jelinek wrote: >On Sat, Feb 13, 2016 at 07:50:25AM +, James Greenhalgh wrote: >> On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote: >> > On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: >> > > >>-

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Sat, Feb 13, 2016 at 07:50:25AM +, James Greenhalgh wrote: > On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote: > > On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > > > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > > > >>+ mode1 = GET_MODE

[PATCH] Fix PR69771, bogus CONST_INT during shift expansion

I am testing the following patch which fixes PR69771 where the code doesn't match the comment before it. We get to expand a QImode << QImode shift but the shift amount was of type int and thus it was expanded as SImode constant. Then /* In case the insn wants input operands in modes

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote: > > I am testing the following patch which fixes PR69771 where the code > doesn't match the comment before it. We get to expand a QImode << QImode > shift but the shift amount was of type int and thus it was expanded > as SImode

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, Feb 12, 2016 at 01:50:08PM +0100, Richard Biener wrote: > > - 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,

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On 02/12/2016 11:46 AM, Richard Biener wrote: On Fri, 12 Feb 2016, Jakub Jelinek wrote: On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote: I am testing the following patch which fixes PR69771 where the code doesn't match the comment before it. We get to expand a QImode <<

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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). > > > >

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, 12 Feb 2016, Jakub Jelinek wrote: > On Fri, Feb 12, 2016 at 11:23:26AM +0100, Richard Biener wrote: > > > > I am testing the following patch which fixes PR69771 where the code > > doesn't match the comment before it. We get to expand a QImode << QImode > > shift but the shift amount was

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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) > { >

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On 02/12/2016 02:47 PM, Richard Biener wrote: Another possibility, only do the convert_modes from VOIDmode for shift_optab_p's xop1, and keep doing what we've done before otherwise. That looks like a very targeted and safe fix indeed. You two can obviously go ahead and sort this out, I'll

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

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 =

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, 12 Feb 2016, Jakub Jelinek wrote: > On Fri, Feb 12, 2016 at 01:50:08PM +0100, Richard Biener wrote: > > > - mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > > > - if (xmode1 != VOIDmode && xmode1 != mode1) > > > + mode1 = GET_MODE (xop1); > > > + if (xmode1 != mode1) >

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

So do you prefer e.g. following? Bootstrapped/regtested on x86_64-linux and i686-linux. - mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; + mode1 = (GET_MODE (xop1) != VOIDmode || canonicalize_op1) + ? GET_MODE (xop1) : mode; Placement of parentheses is wrong for

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > >>+ mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1; > >>if (xmode1 != VOIDmode && xmode1 != mode1) > >> { > > Here, things aren't so

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Fri, Feb 12, 2016 at 05:34:21PM +0100, Jakub Jelinek wrote: > On Fri, Feb 12, 2016 at 03:20:07PM +0100, Bernd Schmidt wrote: > > >>- mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode; > > >>+ mode1 = GET_MODE (xop1) != VOIDmode ? GET_MODE (xop1) : mode1; > > >>if (xmode1 !=

Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion

On Sat, 2016-02-13 at 07:50 +, James Greenhalgh wrote: > > So do you prefer e.g. following? Bootstrapped/regtested on x86_64 > > -linux and > > i686-linux. > > > > 2016-02-12 Jakub Jelinek > > > > PR rtl-optimization/69764 > > PR rtl-optimization/69771 > >