Fix widening multiply expansion (PR rtl-optimization/47299)

Hi!

WIDEN_MULT_EXPR expansion has several issues.  One is that subtarget
shouldn't be used for operand expansion of different mode.
Another one is that if (for whatever reason) we end up with non-folded
WIDEN_MULT_EXPR of two constants up to expansion (don't have testcase, but guess with
many -fno-* options might be possible), the expansion code wouldn't work too
well.  And the last one (triggered on the attached testcase) is that
if we decide to optimize WIDEN_MULT_EXPR with one constant and one
non-constant, the constant needs to be converted from innermode to the
2xwider mode.  For other cases the modes on the insn templates should
ensure the right thing happens.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-01-17  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/47299
	* expr.c (expand_expr_real_2) <case WIDEN_MULT_EXPR>: Don't use
	subtarget.  Use normal multiplication if both operands are
	constants.
	* expmed.c (expand_widening_mult): Don't try to optimize constant
	multiplication if op0 has VOIDmode.  Convert op1 constant to mode
	before using it.

	* gcc.c-torture/execute/pr47299.c: New test.


	Jakub

On Mon, Jan 17, 2011 at 11:20 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> WIDEN_MULT_EXPR expansion has several issues.  One is that subtarget
> shouldn't be used for operand expansion of different mode.
> Another one is that if (for whatever reason) we end up with non-folded
> WIDEN_MULT_EXPR of two constants up to expansion (don't have testcase, but guess with
> many -fno-* options might be possible), the expansion code wouldn't work too

I suppose we simply lack folders (in fold, CCP and VN) and WIDEN_MULT_EXPRs
can at least leak to other functions via IPA inlining and thus
eventually get constants
propagated into it.

> well.  And the last one (triggered on the attached testcase) is that
> if we decide to optimize WIDEN_MULT_EXPR with one constant and one
> non-constant, the constant needs to be converted from innermode to the
> 2xwider mode.  For other cases the modes on the insn templates should
> ensure the right thing happens.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-01-17  Jakub Jelinek  <jakub@redhat.com>
>
>        PR rtl-optimization/47299
>        * expr.c (expand_expr_real_2) <case WIDEN_MULT_EXPR>: Don't use
>        subtarget.  Use normal multiplication if both operands are
>        constants.
>        * expmed.c (expand_widening_mult): Don't try to optimize constant
>        multiplication if op0 has VOIDmode.  Convert op1 constant to mode
>        before using it.
>
>        * gcc.c-torture/execute/pr47299.c: New test.
>
> --- gcc/expr.c.jj       2011-01-04 13:25:20.000000000 +0100
> +++ gcc/expr.c  2011-01-17 12:11:22.215545872 +0100
> @@ -7631,10 +7631,10 @@ expand_expr_real_2 (sepops ops, rtx targ
>              if (optab_handler (this_optab, mode) != CODE_FOR_nothing)
>                {
>                  if (TYPE_UNSIGNED (TREE_TYPE (treeop0)))
> -                   expand_operands (treeop0, treeop1, subtarget, &op0, &op1,
> +                   expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1,
>                                     EXPAND_NORMAL);
>                  else
> -                   expand_operands (treeop0, treeop1, subtarget, &op1, &op0,
> +                   expand_operands (treeop0, treeop1, NULL_RTX, &op1, &op0,
>                                     EXPAND_NORMAL);
>                  goto binop3;
>                }
> @@ -7652,7 +7652,8 @@ expand_expr_real_2 (sepops ops, rtx targ
>          optab other_optab = zextend_p ? smul_widen_optab : umul_widen_optab;
>          this_optab = zextend_p ? umul_widen_optab : smul_widen_optab;
>
> -         if (mode == GET_MODE_2XWIDER_MODE (innermode))
> +         if (mode == GET_MODE_2XWIDER_MODE (innermode)
> +             && TREE_CODE (treeop0) != INTEGER_CST)
>            {
>              if (optab_handler (this_optab, mode) != CODE_FOR_nothing)
>                {
> --- gcc/expmed.c.jj     2010-12-02 13:15:24.000000000 +0100
> +++ gcc/expmed.c        2011-01-17 12:20:28.437673228 +0100
> @@ -3188,12 +3188,17 @@ expand_widening_mult (enum machine_mode
>                      int unsignedp, optab this_optab)
>  {
>   bool speed = optimize_insn_for_speed_p ();
> +  rtx cop1;
>
>   if (CONST_INT_P (op1)
> -      && (INTVAL (op1) >= 0
> +      && GET_MODE (op0) != VOIDmode
> +      && (cop1 = convert_modes (mode, GET_MODE (op0), op1,
> +                               this_optab == umul_widen_optab))
> +      && CONST_INT_P (cop1)
> +      && (INTVAL (cop1) >= 0
>          || GET_MODE_BITSIZE (mode) <= HOST_BITS_PER_WIDE_INT))
>     {
> -      HOST_WIDE_INT coeff = INTVAL (op1);
> +      HOST_WIDE_INT coeff = INTVAL (cop1);
>       int max_cost;
>       enum mult_variant variant;
>       struct algorithm algorithm;
> --- gcc/testsuite/gcc.c-torture/execute/pr47299.c.jj    2011-01-17 12:19:14.120763491 +0100
> +++ gcc/testsuite/gcc.c-torture/execute/pr47299.c       2011-01-17 12:18:55.000000000 +0100
> @@ -0,0 +1,17 @@
> +/* PR rtl-optimization/47299 */
> +
> +extern void abort (void);
> +
> +__attribute__ ((noinline, noclone)) unsigned short
> +foo (unsigned char x)
> +{
> +  return x * 255;
> +}
> +
> +int
> +main ()
> +{
> +  if (foo (0x40) != 0x3fc0)
> +    abort ();
> +  return 0;
> +}
>
>        Jakub
>