diff mbox

Restore widening madd optimisation for fixed-point types

Message ID 87ty4w8cmw.fsf@firetop.home
State New
Headers show

Commit Message

Richard Sandiford Dec. 19, 2011, 9:39 p.m. UTC
The recent(ish) improvements to widening multiplication support have
disabled madd and msub for fixed-point types.  The problem is that the
optab is now chosen based on:

  optype = build_nonstandard_integer_type (from_mode, from_unsigned1);

which is specific to integer types.

The only time optype differs from type1 is when we've switched to using
unsigned types, possibly with a wider mode.  As written, the handling of
mixed signedness really is only suitable for integer types, so this patch
enforces that and makes the call above conditional on it.  It also fixes
the first argument to be the precision rather than the mode.

(As the argument mismatch proved, the precision doesn't actually
matter here; only the signedness does.  I think an equivalent fix
would be to call:

  optype = unsigned_type_for (type1);

That ought to work for fixed point types too, but is a little less
obvious and a little less future-proof.  Since the rest of the block
hasn't been adapted to fixed point types, it didn't seem worth the
confusion.)

Tested on mips-sde-elf, where it fixes gcc.target/mips/dpaq_sa_l_w.c
and gcc.target/mips/dpsq_sa_l_w.c.  OK to install?

Richard


gcc/
	* tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict
	handling of signedness differences to integer types.  Only build
	a new optype if type1 isn't correct.

Comments

Richard Biener Dec. 20, 2011, 9:52 a.m. UTC | #1
On Mon, Dec 19, 2011 at 10:39 PM, Richard Sandiford
<rdsandiford@googlemail.com> wrote:
> The recent(ish) improvements to widening multiplication support have
> disabled madd and msub for fixed-point types.  The problem is that the
> optab is now chosen based on:
>
>  optype = build_nonstandard_integer_type (from_mode, from_unsigned1);
>
> which is specific to integer types.
>
> The only time optype differs from type1 is when we've switched to using
> unsigned types, possibly with a wider mode.  As written, the handling of
> mixed signedness really is only suitable for integer types, so this patch
> enforces that and makes the call above conditional on it.  It also fixes
> the first argument to be the precision rather than the mode.
>
> (As the argument mismatch proved, the precision doesn't actually
> matter here; only the signedness does.  I think an equivalent fix
> would be to call:
>
>  optype = unsigned_type_for (type1);
>
> That ought to work for fixed point types too, but is a little less
> obvious and a little less future-proof.  Since the rest of the block
> hasn't been adapted to fixed point types, it didn't seem worth the
> confusion.)
>
> Tested on mips-sde-elf, where it fixes gcc.target/mips/dpaq_sa_l_w.c
> and gcc.target/mips/dpsq_sa_l_w.c.  OK to install?

Ok with ...

> Richard
>
>
> gcc/
>        * tree-ssa-math-opts.c (convert_plusminus_to_widen): Restrict
>        handling of signedness differences to integer types.  Only build
>        a new optype if type1 isn't correct.
>
> Index: gcc/tree-ssa-math-opts.c
> ===================================================================
> --- gcc/tree-ssa-math-opts.c    2011-12-19 21:18:43.000000000 +0000
> +++ gcc/tree-ssa-math-opts.c    2011-12-19 21:23:12.000000000 +0000
> @@ -2304,10 +2304,13 @@ convert_plusminus_to_widen (gimple_stmt_
>   from_mode = TYPE_MODE (type1);
>   from_unsigned1 = TYPE_UNSIGNED (type1);
>   from_unsigned2 = TYPE_UNSIGNED (type2);
> +  optype = type1;
>
>   /* There's no such thing as a mixed sign madd yet, so use a wider mode.  */
>   if (from_unsigned1 != from_unsigned2)
>     {
> +      if (TREE_CODE (type) != INTEGER_TYPE)
> +       return false;

using INTEGRAL_TYPE_P (type) instead.  That way it still works
when applied to enum type multiplications.

Thanks,
Richard.

>       /* We can use a signed multiply with unsigned types as long as
>         there is a wider mode to use, or it is the smaller of the two
>         types that is unsigned.  Note that type1 >= type2, always.  */
> @@ -2322,6 +2325,8 @@ convert_plusminus_to_widen (gimple_stmt_
>        }
>
>       from_unsigned1 = from_unsigned2 = false;
> +      optype = build_nonstandard_integer_type (GET_MODE_PRECISION (from_mode),
> +                                              false);
>     }
>
>   /* If there was a conversion between the multiply and addition
> @@ -2355,7 +2360,6 @@ convert_plusminus_to_widen (gimple_stmt_
>   /* Verify that the machine can perform a widening multiply
>      accumulate in this mode/signedness combination, otherwise
>      this transformation is likely to pessimize code.  */
> -  optype = build_nonstandard_integer_type (from_mode, from_unsigned1);
>   this_optab = optab_for_tree_code (wmult_code, optype, optab_default);
>   handler = find_widening_optab_handler_and_mode (this_optab, to_mode,
>                                                  from_mode, 0, &actual_mode);
diff mbox

Patch

Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c	2011-12-19 21:18:43.000000000 +0000
+++ gcc/tree-ssa-math-opts.c	2011-12-19 21:23:12.000000000 +0000
@@ -2304,10 +2304,13 @@  convert_plusminus_to_widen (gimple_stmt_
   from_mode = TYPE_MODE (type1);
   from_unsigned1 = TYPE_UNSIGNED (type1);
   from_unsigned2 = TYPE_UNSIGNED (type2);
+  optype = type1;
 
   /* There's no such thing as a mixed sign madd yet, so use a wider mode.  */
   if (from_unsigned1 != from_unsigned2)
     {
+      if (TREE_CODE (type) != INTEGER_TYPE)
+	return false;
       /* We can use a signed multiply with unsigned types as long as
 	 there is a wider mode to use, or it is the smaller of the two
 	 types that is unsigned.  Note that type1 >= type2, always.  */
@@ -2322,6 +2325,8 @@  convert_plusminus_to_widen (gimple_stmt_
 	}
 
       from_unsigned1 = from_unsigned2 = false;
+      optype = build_nonstandard_integer_type (GET_MODE_PRECISION (from_mode),
+					       false);
     }
 
   /* If there was a conversion between the multiply and addition
@@ -2355,7 +2360,6 @@  convert_plusminus_to_widen (gimple_stmt_
   /* Verify that the machine can perform a widening multiply
      accumulate in this mode/signedness combination, otherwise
      this transformation is likely to pessimize code.  */
-  optype = build_nonstandard_integer_type (from_mode, from_unsigned1);
   this_optab = optab_for_tree_code (wmult_code, optype, optab_default);
   handler = find_widening_optab_handler_and_mode (this_optab, to_mode,
 						  from_mode, 0, &actual_mode);