Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-09-07 Thread Richard Earnshaw
On 20/08/12 12:36, Richard Guenther wrote: On Fri, Aug 17, 2012 at 7:05 PM, Richard Earnshaw rearn...@arm.com wrote: On 17/08/12 16:20, Richard Earnshaw wrote: Ok, in which case we have to give is_widening_mult_rhs_p enough smarts to not strip (s32)u32 and return u32. I'll have

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-09-07 Thread Richard Guenther
On Fri, Sep 7, 2012 at 10:35 AM, Richard Earnshaw rearn...@arm.com wrote: On 20/08/12 12:36, Richard Guenther wrote: On Fri, Aug 17, 2012 at 7:05 PM, Richard Earnshaw rearn...@arm.com wrote: On 17/08/12 16:20, Richard Earnshaw wrote: Ok, in which case we have to give is_widening_mult_rhs_p

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-20 Thread Richard Guenther
On Fri, Aug 17, 2012 at 7:05 PM, Richard Earnshaw rearn...@arm.com wrote: On 17/08/12 16:20, Richard Earnshaw wrote: Ok, in which case we have to give is_widening_mult_rhs_p enough smarts to not strip (s32)u32 and return u32. I'll have another think about it. Take two. This

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-20 Thread Tobias Burnus
Hi Richard, your patch fails here; I get the build failure: /projects/tob/gcc-git/gcc/gcc/tree-ssa-math-opts.c: In function ‘bool is_widening_mult_rhs_p(tree, tree, tree_node**, tree_node**)’: /projects/tob/gcc-git/gcc/gcc/tree-ssa-math-opts.c:2014:18: error: variable ‘rhs_code’ set but not

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-20 Thread Richard Earnshaw
On 20/08/12 15:01, Tobias Burnus wrote: Hi Richard, your patch fails here; I get the build failure: /projects/tob/gcc-git/gcc/gcc/tree-ssa-math-opts.c: In function ‘bool is_widening_mult_rhs_p(tree, tree, tree_node**, tree_node**)’:

[patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Richard Earnshaw
PR54295 shows that widening multiply-accumulate operations can end up with incorrect code. The path to the failure is as follows 1) Compiler detects a widening multiply operation and generates the correct widening-multiply operation. This involves stripping off the widening cast to leave the

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Andrew Stubbs
On 17/08/12 15:04, Richard Earnshaw wrote: The fix is to make is_widening_mult_p note that it has been called with a WIDEN_MULT_EXPR and rather than decompose the operands again, to simply extract the existing operands, which have already been formulated correctly for a widening multiply

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Richard Earnshaw
On 17/08/12 15:22, Andrew Stubbs wrote: On 17/08/12 15:04, Richard Earnshaw wrote: The fix is to make is_widening_mult_p note that it has been called with a WIDEN_MULT_EXPR and rather than decompose the operands again, to simply extract the existing operands, which have already been formulated

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Andrew Stubbs
On 17/08/12 15:31, Richard Earnshaw wrote: On 17/08/12 15:22, Andrew Stubbs wrote: On 17/08/12 15:04, Richard Earnshaw wrote: The fix is to make is_widening_mult_p note that it has been called with a WIDEN_MULT_EXPR and rather than decompose the operands again, to simply extract the existing

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Richard Earnshaw
On 17/08/12 15:39, Andrew Stubbs wrote: On 17/08/12 15:31, Richard Earnshaw wrote: On 17/08/12 15:22, Andrew Stubbs wrote: On 17/08/12 15:04, Richard Earnshaw wrote: The fix is to make is_widening_mult_p note that it has been called with a WIDEN_MULT_EXPR and rather than decompose the

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Andrew Stubbs
On 17/08/12 15:47, Richard Earnshaw wrote: If we don't have a 16x16-64 mult operation then after step 1 we'll still have a MULT_EXPR, not a WIDEN_MULT_EXPR, so when we reach step2 there's nothing to short circuit. Unless, of course, you're expecting us to get step1 - 16x16-32 widen mult step2

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Richard Earnshaw
On 17/08/12 16:06, Andrew Stubbs wrote: On 17/08/12 15:47, Richard Earnshaw wrote: If we don't have a 16x16-64 mult operation then after step 1 we'll still have a MULT_EXPR, not a WIDEN_MULT_EXPR, so when we reach step2 there's nothing to short circuit. Unless, of course, you're expecting us

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Richard Earnshaw
On 17/08/12 16:20, Richard Earnshaw wrote: Ok, in which case we have to give is_widening_mult_rhs_p enough smarts to not strip (s32)u32 and return u32. I'll have another think about it. Take two. This version should address your concerns about handling (u32)u16 *

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Andrew Stubbs
On 17/08/12 16:20, Richard Earnshaw wrote: No, given a u16xu16-u64 operation in the code, and that the arch doesn't have such an opcode, I'd expect to get step1 - (u32)u16 x (u32)u16 - u64 Hmm, I would have thought that would be more costly than (u64)(u16 x u16 - u32) You might be

Re: [patch, tree-ssa] PR54295 Incorrect value extension in widening multiply-accumulate

2012-08-17 Thread Andrew Stubbs
On 17/08/12 18:05, Richard Earnshaw wrote: Take two. This version should address your concerns about handling (u32)u16 * (u32)u16 - u64 We now look at each operand directly, but when doing so we check whether the operand is the same size as the result or not. When it is, we can strip