Re: [PATCH (1/7)] New optab framework for widening multiplies

[Andrew Stubbs <ams@codesourcery.com>]

On 22/07/11 13:34, Bernd Schmidt wrote:
>> @@ -1242,7 +1242,8 @@ expand_binop_directly (enum machine_mode mode, optab binoptab,
>> >    		rtx target, int unsignedp, enum optab_methods methods,
>> >    		rtx last)
>> >    {
>> >  -  enum insn_code icode = optab_handler (binoptab, mode);
>> >  +  enum machine_mode from_mode = GET_MODE (op0);
>> >  +  enum insn_code icode = widening_optab_handler (binoptab, mode, from_mode);
> Please add a new function along the lines of
>
> enum machine_mode
> widened_mode (enum machine_mode to_mode, rtx op0, rtx op1)
> {
>    if (GET_MODE (op1) == VOIDmode)
>      return GET_MODE (op0);
>    gcc_assert (GET_MODE (op0) == GET_MODE (op1);
>    return GET_MODE (op0);
> }
>
> I'll want to extend this at some point to allow widening multiplies
> where only one operand is widened (with a new set of optabs).

Sorry, I don't quite understand what you're getting at here?

expand_binop_directly is only ever used, I think, when the tree 
optimizer has already identified what insn to use. Both before and after 
my patch, the tree-cfg gimple verification requires that both op0 and 
op1 are the same mode, and non-widening operation are always he same 
mode, so I think my code is perfectly adequate. Is that not so?

If you want to add support for machine instructions that only widen one 
input, then that's surely a separate problem? If the target mode is 
smaller than the combined size of the inputs, then the changes to the 
widening_mul pass would be non-trivial.

If the point is just to be absolutely certain that the inputs are valid 
then I'm happy to add the function. BTW, did you mean the have the 
unused parameter?

Andrew