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

[Andrew Stubbs <ams@codesourcery.com>]

[-- Attachment #1: Type: text/plain, Size: 702 bytes --]

On 22/07/11 16:34, Andrew Stubbs wrote:
> On 22/07/11 14:28, Bernd Schmidt wrote:
>> Oh well, let's shelve it and do it later.
>
> Here's an updated patch with the formatting problem you found fixed.

I've just committed an updated version of this patch (attached).

I found a number of subtle bugs while I was testing, and these have now 
been corrected. In particular, I found that VOIDmode constants were not 
handled correctly; I've added a function "widened_mode" along the lines 
originally suggested by Benrd to deal with this. I also found one case 
where different code was produced to previously, although it was 
actually corrected later in the patch series I've fixed it here now.

Andrew


[-- Attachment #2: widening-multiplies-1.patch --]
[-- Type: text/x-patch, Size: 15169 bytes --]

2011-08-19  Andrew Stubbs  <ams@codesourcery.com>

	gcc/
	* expr.c (expand_expr_real_2): Use widening_optab_handler.
	* genopinit.c (optabs): Use set_widening_optab_handler for $N.
	(gen_insn): $N now means $a must be wider than $b, not consecutive.
	* optabs.c (widened_mode): New function.
	(expand_widen_pattern_expr): Use widening_optab_handler.
	(expand_binop_directly): Likewise.
	(expand_binop): Likewise.
	* optabs.h (widening_optab_handlers): New struct.
	(optab_d): New member, 'widening'.
	(widening_optab_handler): New function.
	(set_widening_optab_handler): New function.
	* tree-ssa-math-opts.c (convert_mult_to_widen): Use
	widening_optab_handler.
	(convert_plusminus_to_widen): Likewise.

--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -8005,7 +8005,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,
 	  this_optab = usmul_widen_optab;
 	  if (mode == GET_MODE_2XWIDER_MODE (innermode))
 	    {
-	      if (optab_handler (this_optab, mode) != CODE_FOR_nothing)
+	      if (widening_optab_handler (this_optab, mode, innermode)
+		    != CODE_FOR_nothing)
 		{
 		  if (TYPE_UNSIGNED (TREE_TYPE (treeop0)))
 		    expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1,
@@ -8032,7 +8033,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,
 	  if (mode == GET_MODE_2XWIDER_MODE (innermode)
 	      && TREE_CODE (treeop0) != INTEGER_CST)
 	    {
-	      if (optab_handler (this_optab, mode) != CODE_FOR_nothing)
+	      if (widening_optab_handler (this_optab, mode, innermode)
+		    != CODE_FOR_nothing)
 		{
 		  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1,
 				   EXPAND_NORMAL);
@@ -8040,7 +8042,8 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode,
 					       unsignedp, this_optab);
 		  return REDUCE_BIT_FIELD (temp);
 		}
-	      if (optab_handler (other_optab, mode) != CODE_FOR_nothing
+	      if (widening_optab_handler (other_optab, mode, innermode)
+		    != CODE_FOR_nothing
 		  && innermode == word_mode)
 		{
 		  rtx htem, hipart;
--- a/gcc/genopinit.c
+++ b/gcc/genopinit.c
@@ -46,10 +46,12 @@ along with GCC; see the file COPYING3.  If not see
    used.  $A and $B are replaced with the full name of the mode; $a and $b
    are replaced with the short form of the name, as above.
 
-   If $N is present in the pattern, it means the two modes must be consecutive
-   widths in the same mode class (e.g, QImode and HImode).  $I means that
-   only full integer modes should be considered for the next mode, and $F
-   means that only float modes should be considered.
+   If $N is present in the pattern, it means the two modes must be in
+   the same mode class, and $b must be greater than $a (e.g, QImode
+   and HImode).
+
+   $I means that only full integer modes should be considered for the
+   next mode, and $F means that only float modes should be considered.
    $P means that both full and partial integer modes should be considered.
    $Q means that only fixed-point modes should be considered.
 
@@ -99,17 +101,17 @@ static const char * const optabs[] =
   "set_optab_handler (smulv_optab, $A, CODE_FOR_$(mulv$I$a3$))",
   "set_optab_handler (umul_highpart_optab, $A, CODE_FOR_$(umul$a3_highpart$))",
   "set_optab_handler (smul_highpart_optab, $A, CODE_FOR_$(smul$a3_highpart$))",
-  "set_optab_handler (smul_widen_optab, $B, CODE_FOR_$(mul$a$b3$)$N)",
-  "set_optab_handler (umul_widen_optab, $B, CODE_FOR_$(umul$a$b3$)$N)",
-  "set_optab_handler (usmul_widen_optab, $B, CODE_FOR_$(usmul$a$b3$)$N)",
-  "set_optab_handler (smadd_widen_optab, $B, CODE_FOR_$(madd$a$b4$)$N)",
-  "set_optab_handler (umadd_widen_optab, $B, CODE_FOR_$(umadd$a$b4$)$N)",
-  "set_optab_handler (ssmadd_widen_optab, $B, CODE_FOR_$(ssmadd$a$b4$)$N)",
-  "set_optab_handler (usmadd_widen_optab, $B, CODE_FOR_$(usmadd$a$b4$)$N)",
-  "set_optab_handler (smsub_widen_optab, $B, CODE_FOR_$(msub$a$b4$)$N)",
-  "set_optab_handler (umsub_widen_optab, $B, CODE_FOR_$(umsub$a$b4$)$N)",
-  "set_optab_handler (ssmsub_widen_optab, $B, CODE_FOR_$(ssmsub$a$b4$)$N)",
-  "set_optab_handler (usmsub_widen_optab, $B, CODE_FOR_$(usmsub$a$b4$)$N)",
+  "set_widening_optab_handler (smul_widen_optab, $B, $A, CODE_FOR_$(mul$a$b3$)$N)",
+  "set_widening_optab_handler (umul_widen_optab, $B, $A, CODE_FOR_$(umul$a$b3$)$N)",
+  "set_widening_optab_handler (usmul_widen_optab, $B, $A, CODE_FOR_$(usmul$a$b3$)$N)",
+  "set_widening_optab_handler (smadd_widen_optab, $B, $A, CODE_FOR_$(madd$a$b4$)$N)",
+  "set_widening_optab_handler (umadd_widen_optab, $B, $A, CODE_FOR_$(umadd$a$b4$)$N)",
+  "set_widening_optab_handler (ssmadd_widen_optab, $B, $A, CODE_FOR_$(ssmadd$a$b4$)$N)",
+  "set_widening_optab_handler (usmadd_widen_optab, $B, $A, CODE_FOR_$(usmadd$a$b4$)$N)",
+  "set_widening_optab_handler (smsub_widen_optab, $B, $A, CODE_FOR_$(msub$a$b4$)$N)",
+  "set_widening_optab_handler (umsub_widen_optab, $B, $A, CODE_FOR_$(umsub$a$b4$)$N)",
+  "set_widening_optab_handler (ssmsub_widen_optab, $B, $A, CODE_FOR_$(ssmsub$a$b4$)$N)",
+  "set_widening_optab_handler (usmsub_widen_optab, $B, $A, CODE_FOR_$(usmsub$a$b4$)$N)",
   "set_optab_handler (sdiv_optab, $A, CODE_FOR_$(div$a3$))",
   "set_optab_handler (ssdiv_optab, $A, CODE_FOR_$(ssdiv$Q$a3$))",
   "set_optab_handler (sdivv_optab, $A, CODE_FOR_$(div$V$I$a3$))",
@@ -305,7 +307,7 @@ gen_insn (rtx insn)
     {
       int force_float = 0, force_int = 0, force_partial_int = 0;
       int force_fixed = 0;
-      int force_consec = 0;
+      int force_wider = 0;
       int matches = 1;
 
       for (pp = optabs[pindex]; pp[0] != '$' || pp[1] != '('; pp++)
@@ -323,7 +325,7 @@ gen_insn (rtx insn)
 	    switch (*++pp)
 	      {
 	      case 'N':
-		force_consec = 1;
+		force_wider = 1;
 		break;
 	      case 'I':
 		force_int = 1;
@@ -392,7 +394,10 @@ gen_insn (rtx insn)
 			    || mode_class[i] == MODE_VECTOR_FRACT
 			    || mode_class[i] == MODE_VECTOR_UFRACT
 			    || mode_class[i] == MODE_VECTOR_ACCUM
-			    || mode_class[i] == MODE_VECTOR_UACCUM))
+			    || mode_class[i] == MODE_VECTOR_UACCUM)
+			&& (! force_wider
+			    || *pp == 'a'
+			    || m1 < i))
 		      break;
 		  }
 
@@ -412,8 +417,7 @@ gen_insn (rtx insn)
 	}
 
       if (matches && pp[0] == '$' && pp[1] == ')'
-	  && *np == 0
-	  && (! force_consec || (int) GET_MODE_WIDER_MODE(m1) == m2))
+	  && *np == 0)
 	break;
     }
 
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -225,6 +225,30 @@ add_equal_note (rtx insns, rtx target, enum rtx_code code, rtx op0, rtx op1)
   return 1;
 }
 \f
+/* Given two input operands, OP0 and OP1, determine what the correct from_mode
+   for a widening operation would be.  In most cases this would be OP0, but if
+   that's a constant it'll be VOIDmode, which isn't useful.  */
+
+static enum machine_mode
+widened_mode (enum machine_mode to_mode, rtx op0, rtx op1)
+{
+  enum machine_mode m0 = GET_MODE (op0);
+  enum machine_mode m1 = GET_MODE (op1);
+  enum machine_mode result;
+
+  if (m0 == VOIDmode && m1 == VOIDmode)
+    return to_mode;
+  else if (m0 == VOIDmode || GET_MODE_SIZE (m0) < GET_MODE_SIZE (m1))
+    result = m1;
+  else
+    result = m0;
+
+  if (GET_MODE_SIZE (result) > GET_MODE_SIZE (to_mode))
+    return to_mode;
+
+  return result;
+}
+\f
 /* Widen OP to MODE and return the rtx for the widened operand.  UNSIGNEDP
    says whether OP is signed or unsigned.  NO_EXTEND is nonzero if we need
    not actually do a sign-extend or zero-extend, but can leave the
@@ -515,8 +539,8 @@ expand_widen_pattern_expr (sepops ops, rtx op0, rtx op1, rtx wide_op,
     optab_for_tree_code (ops->code, TREE_TYPE (oprnd0), optab_default);
   if (ops->code == WIDEN_MULT_PLUS_EXPR
       || ops->code == WIDEN_MULT_MINUS_EXPR)
-    icode = optab_handler (widen_pattern_optab,
-			   TYPE_MODE (TREE_TYPE (ops->op2)));
+    icode = widening_optab_handler (widen_pattern_optab,
+				    TYPE_MODE (TREE_TYPE (ops->op2)), tmode0);
   else
     icode = optab_handler (widen_pattern_optab, tmode0);
   gcc_assert (icode != CODE_FOR_nothing);
@@ -1242,7 +1266,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 = widened_mode (mode, op0, op1);
+  enum insn_code icode = widening_optab_handler (binoptab, mode, from_mode);
   enum machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
   enum machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
   enum machine_mode mode0, mode1, tmp_mode;
@@ -1389,7 +1414,9 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
   /* If we can do it with a three-operand insn, do so.  */
 
   if (methods != OPTAB_MUST_WIDEN
-      && optab_handler (binoptab, mode) != CODE_FOR_nothing)
+      && widening_optab_handler (binoptab, mode,
+				 widened_mode (mode, op0, op1))
+	    != CODE_FOR_nothing)
     {
       temp = expand_binop_directly (mode, binoptab, op0, op1, target,
 				    unsignedp, methods, last);
@@ -1429,8 +1456,9 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
 
   if (binoptab == smul_optab
       && GET_MODE_2XWIDER_MODE (mode) != VOIDmode
-      && (optab_handler ((unsignedp ? umul_widen_optab : smul_widen_optab),
-			 GET_MODE_2XWIDER_MODE (mode))
+      && (widening_optab_handler ((unsignedp ? umul_widen_optab
+					     : smul_widen_optab),
+				  GET_MODE_2XWIDER_MODE (mode), mode)
 	  != CODE_FOR_nothing))
     {
       temp = expand_binop (GET_MODE_2XWIDER_MODE (mode),
@@ -1460,9 +1488,10 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
 	if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing
 	    || (binoptab == smul_optab
 		&& GET_MODE_WIDER_MODE (wider_mode) != VOIDmode
-		&& (optab_handler ((unsignedp ? umul_widen_optab
-				    : smul_widen_optab),
-				   GET_MODE_WIDER_MODE (wider_mode))
+		&& (widening_optab_handler ((unsignedp ? umul_widen_optab
+						       : smul_widen_optab),
+					    GET_MODE_WIDER_MODE (wider_mode),
+					    mode)
 		    != CODE_FOR_nothing)))
 	  {
 	    rtx xop0 = op0, xop1 = op1;
@@ -1895,8 +1924,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
       && optab_handler (add_optab, word_mode) != CODE_FOR_nothing)
     {
       rtx product = NULL_RTX;
-
-      if (optab_handler (umul_widen_optab, mode) != CODE_FOR_nothing)
+      if (widening_optab_handler (umul_widen_optab, mode, word_mode)
+	    != CODE_FOR_nothing)
 	{
 	  product = expand_doubleword_mult (mode, op0, op1, target,
 					    true, methods);
@@ -1905,7 +1934,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
 	}
 
       if (product == NULL_RTX
-	  && optab_handler (smul_widen_optab, mode) != CODE_FOR_nothing)
+	  && widening_optab_handler (smul_widen_optab, mode, word_mode)
+		!= CODE_FOR_nothing)
 	{
 	  product = expand_doubleword_mult (mode, op0, op1, target,
 					    false, methods);
@@ -1997,6 +2027,8 @@ expand_binop (enum machine_mode mode, optab binoptab, rtx op0, rtx op1,
 	   wider_mode = GET_MODE_WIDER_MODE (wider_mode))
 	{
 	  if (optab_handler (binoptab, wider_mode) != CODE_FOR_nothing
+	      || widening_optab_handler (binoptab, wider_mode, mode)
+		  != CODE_FOR_nothing
 	      || (methods == OPTAB_LIB
 		  && optab_libfunc (binoptab, wider_mode)))
 	    {
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -42,6 +42,11 @@ struct optab_handlers
   int insn_code;
 };
 
+struct widening_optab_handlers
+{
+  struct optab_handlers handlers[NUM_MACHINE_MODES][NUM_MACHINE_MODES];
+};
+
 struct optab_d
 {
   enum rtx_code code;
@@ -50,6 +55,7 @@ struct optab_d
   void (*libcall_gen)(struct optab_d *, const char *name, char suffix,
 		      enum machine_mode);
   struct optab_handlers handlers[NUM_MACHINE_MODES];
+  struct widening_optab_handlers *widening;
 };
 typedef struct optab_d * optab;
 
@@ -879,6 +885,23 @@ optab_handler (optab op, enum machine_mode mode)
 			   + (int) CODE_FOR_nothing);
 }
 
+/* Like optab_handler, but for widening_operations that have a TO_MODE and
+  a FROM_MODE.  */
+
+static inline enum insn_code
+widening_optab_handler (optab op, enum machine_mode to_mode,
+			enum machine_mode from_mode)
+{
+  if (to_mode == from_mode || from_mode == VOIDmode)
+    return optab_handler (op, to_mode);
+
+  if (op->widening)
+    return (enum insn_code) (op->widening->handlers[(int) to_mode][(int) from_mode].insn_code
+			     + (int) CODE_FOR_nothing);
+
+  return CODE_FOR_nothing;
+}
+
 /* Record that insn CODE should be used to implement mode MODE of OP.  */
 
 static inline void
@@ -887,6 +910,26 @@ set_optab_handler (optab op, enum machine_mode mode, enum insn_code code)
   op->handlers[(int) mode].insn_code = (int) code - (int) CODE_FOR_nothing;
 }
 
+/* Like set_optab_handler, but for widening operations that have a TO_MODE
+   and a FROM_MODE.  */
+
+static inline void
+set_widening_optab_handler (optab op, enum machine_mode to_mode,
+			    enum machine_mode from_mode, enum insn_code code)
+{
+  if (to_mode == from_mode)
+    set_optab_handler (op, to_mode, code);
+  else
+    {
+      if (op->widening == NULL)
+	op->widening = (struct widening_optab_handlers *)
+	      xcalloc (1, sizeof (struct widening_optab_handlers));
+
+      op->widening->handlers[(int) to_mode][(int) from_mode].insn_code
+	  = (int) code - (int) CODE_FOR_nothing;
+    }
+}
+
 /* Return the insn used to perform conversion OP from mode FROM_MODE
    to mode TO_MODE; return CODE_FOR_nothing if the target does not have
    such an insn.  */
--- a/gcc/tree-ssa-math-opts.c
+++ b/gcc/tree-ssa-math-opts.c
@@ -2056,6 +2056,8 @@ convert_mult_to_widen (gimple stmt)
 {
   tree lhs, rhs1, rhs2, type, type1, type2;
   enum insn_code handler;
+  enum machine_mode to_mode, from_mode;
+  optab op;
 
   lhs = gimple_assign_lhs (stmt);
   type = TREE_TYPE (lhs);
@@ -2065,12 +2067,17 @@ convert_mult_to_widen (gimple stmt)
   if (!is_widening_mult_p (stmt, &type1, &rhs1, &type2, &rhs2))
     return false;
 
+  to_mode = TYPE_MODE (type);
+  from_mode = TYPE_MODE (type1);
+
   if (TYPE_UNSIGNED (type1) && TYPE_UNSIGNED (type2))
-    handler = optab_handler (umul_widen_optab, TYPE_MODE (type));
+    op = umul_widen_optab;
   else if (!TYPE_UNSIGNED (type1) && !TYPE_UNSIGNED (type2))
-    handler = optab_handler (smul_widen_optab, TYPE_MODE (type));
+    op = smul_widen_optab;
   else
-    handler = optab_handler (usmul_widen_optab, TYPE_MODE (type));
+    op = usmul_widen_optab;
+
+  handler = widening_optab_handler (op, to_mode, from_mode);
 
   if (handler == CODE_FOR_nothing)
     return false;
@@ -2172,7 +2179,8 @@ convert_plusminus_to_widen (gimple_stmt_iterator *gsi, gimple stmt,
      accumulate in this mode/signedness combination, otherwise
      this transformation is likely to pessimize code.  */
   this_optab = optab_for_tree_code (wmult_code, type1, optab_default);
-  if (optab_handler (this_optab, TYPE_MODE (type)) == CODE_FOR_nothing)
+  if (widening_optab_handler (this_optab, TYPE_MODE (type), TYPE_MODE (type1))
+	== CODE_FOR_nothing)
     return false;
 
   /* ??? May need some type verification here?  */