[PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Andrew Stubbs
This patch should fix the bug in pr50193. The problem is that the arith_shiftsi pattern accepted any arbitrary constant as the shift amount (via the shift_amount_operand predicate) where in fact the constant must be in the range 0..32. This patch fixes the problem by merely checking that the

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Joseph S. Myers
On Thu, 1 Sep 2011, Andrew Stubbs wrote: > This patch fixes the problem by merely checking that the constant is positive. > I've confirmed that values larger than the mode-size are not a problem because > the compiler optimizes those away earlier, even at -O0. Do you mean that you have observed f

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Andrew Stubbs
On 01/09/11 14:21, Joseph S. Myers wrote: On Thu, 1 Sep 2011, Andrew Stubbs wrote: This patch fixes the problem by merely checking that the constant is positive. I've confirmed that values larger than the mode-size are not a problem because the compiler optimizes those away earlier, even at -O0

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Andrew Stubbs
On 01/09/11 16:26, Andrew Stubbs wrote: OK, fair enough, redundant or not, here's a patch with belt and braces. OK now? And again, with the patch Andrew 2011-09-01 Andrew Stubbs gcc/ * config/arm/predicates.md (shift_amount_operand): Ensure shift amount is in the range 1..mode_siz

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Jakub Jelinek
On Thu, Sep 01, 2011 at 04:29:25PM +0100, Andrew Stubbs wrote: > On 01/09/11 16:26, Andrew Stubbs wrote: > >OK, fair enough, redundant or not, here's a patch with belt and braces. > > > >OK now? > > And again, with the patch > > Andrew > 2011-09-01 Andrew Stubbs > > gcc/ > *

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-01 Thread Andrew Stubbs
On 01/09/11 16:51, Jakub Jelinek wrote: IN_RANGE (INTVAL (op), 0, GET_MODE_PRECISION (mode) - 1) ? Ok, I can make that change. 1) shift by 0 is well defined (though not sure if arm backend supports it) Yeah, I suppose I could allow 0, but I don't know why it would be helpful? I mean, i

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-05 Thread Andrew Stubbs
On 01/09/11 17:21, Andrew Stubbs wrote: I wasn't sure how to find the mode of shift operand in the predicate though, so I've assumed they're always the same size. How would one find the proper mode in a predicate? OK, no reply, so I'm just going to assume we're dealing with 32-bit registers.

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-07 Thread Ramana Radhakrishnan
On 5 September 2011 18:07, Andrew Stubbs wrote: > On 01/09/11 17:21, Andrew Stubbs wrote: >> >> I wasn't sure how to find the mode of shift operand in the predicate >> though, so I've assumed they're always the same size. How would one find >> the proper mode in a predicate? > > OK, no reply, so I

Re: [PATCH][ARM] pr50193: ICE on a | (b << negative-constant)

2011-09-07 Thread Richard Earnshaw
On 01/09/11 16:51, Jakub Jelinek wrote: > 1) shift by 0 is well defined (though not sure if arm backend >supports it) The canonical form of ( (x) (const_int 0)) is just (x) So while it's well defined, it's also useless.