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

[Richard Earnshaw <rearnsha at arm dot com>]

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 operands again, to
>>>> simply extract the existing operands, which have already been formulated
>>>> correctly for a widening multiply operation.
>>>
>>> As long as the existing test cases work, I think the only problem with
>>> this idea is if some architecture has a wider range of widening
>>> multiply-and-accumulate than it does plain widening multiply.
>>
>> But surely in that case step1 wouldn't have applied and we'd then be
>> looking at a MULT_EXPR not a WIDEN_MULT_EXPR.
> 
> Not necessarily.
> 
> For example, it will use a 32x32->64 signed widening multiply for 16 bit 
> unsigned data if there is no unsigned 16x16->64 option. Hypothetically, 
> if there happened to be an unsigned 16x16->64 multiply-and-accumulate 
> then you'd end up masking it.
> 
> Like I said though, cross that bridge if it ever comes to it.
> 
> Andrew
> 

You've lost me.

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 -> widen64(step1) + acc64

But even this is only safe iff widen64 is the same type of widening as
the original widening step, and we determine that by looking at the
operands of the widening mult, not at any casts on them.

The key thing is that the type of multiply that we want is based on the
types of the operands, not on the type of the result.  So it's essential
we don't strip type conversions beyond the widening conversion.

R.