diff mbox series

[10/nn] Widening optab cleanup

Message ID 87po9ew1jf.fsf@linaro.org
State New
Headers show
Series [10/nn] Widening optab cleanup | expand

Commit Message

Richard Sandiford Oct. 23, 2017, 11:23 a.m. UTC
widening_optab_handler had the comment:

      /* ??? Why does find_widening_optab_handler_and_mode attempt to
         widen things that can't be widened?  E.g. add_optab... */
      if (op > LAST_CONV_OPTAB)
        return CODE_FOR_nothing;

I think it comes from expand_binop using
find_widening_optab_handler_and_mode for two things: to test whether
a "normal" optab like add_optab is supported for a standard binary
operation and to test whether a "convert" optab is supported for a
widening operation like umul_widen_optab.  In the former case from_mode
and to_mode must be the same, in the latter from_mode must be narrower
than to_mode.

For the former case, find_widening_optab_handler_and_mode is only really
testing the modes that are passed in.  permit_non_widening must be true
here.

For the latter case, find_widening_optab_handler_and_mode should only
really consider new from_modes that are wider than the original
from_mode and narrower than the original to_mode.  Logically
permit_non_widening should be false, since widening optabs aren't
supposed to take operands that are the same width as the destination.
We get away with permit_non_widening being true because no target
would/should define a widening .md pattern with matching modes.

But really, it seems better for expand_binop to handle these two
cases itself rather than pushing them down.  With that change,
find_widening_optab_handler_and_mode is only ever called with
permit_non_widening set to false and is only ever called with
a "proper" convert optab.  We then no longer need widening_optab_handler,
we can just use convert_optab_handler directly.

The patch also passes the instruction code down to expand_binop_directly.
This should be more efficient and removes an extra call to
find_widening_optab_handler_and_mode.


2017-10-23  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* optabs-query.h (convert_optab_p): New function, split out from...
	(convert_optab_handler): ...here.
	(widening_optab_handler): Delete.
	(find_widening_optab_handler): Remove permit_non_widening parameter.
	(find_widening_optab_handler_and_mode): Likewise.  Provide an
	override that operates on mode class wrappers.
	* optabs-query.c (widening_optab_handler): Delete.
	(find_widening_optab_handler_and_mode): Remove permit_non_widening
	parameter.  Assert that the two modes are the same class and that
	the "from" mode is narrower than the "to" mode.  Use
	convert_optab_handler instead of widening_optab_handler.
	* expmed.c (expmed_mult_highpart_optab): Use convert_optab_handler
	instead of widening_optab_handler.
	* expr.c (expand_expr_real_2): Update calls to
	find_widening_optab_handler.
	* optabs.c (expand_widen_pattern_expr): Likewise.
	(expand_binop_directly): Take the insn_code as a parameter.
	(expand_binop): Only call find_widening_optab_handler for
	conversion optabs; use optab_handler otherwise.  Update calls
	to find_widening_optab_handler and expand_binop_directly.
	Use convert_optab_handler instead of widening_optab_handler.
	* tree-ssa-math-opts.c (convert_mult_to_widen): Update calls to
	find_widening_optab_handler and use scalar_mode rather than
	machine_mode.
	(convert_plusminus_to_widen): Likewise.

Comments

Jeff Law Oct. 30, 2017, 6:25 p.m. UTC | #1
On 10/23/2017 05:23 AM, Richard Sandiford wrote:
> widening_optab_handler had the comment:

> 

>       /* ??? Why does find_widening_optab_handler_and_mode attempt to

>          widen things that can't be widened?  E.g. add_optab... */

>       if (op > LAST_CONV_OPTAB)

>         return CODE_FOR_nothing;

> 

> I think it comes from expand_binop using

> find_widening_optab_handler_and_mode for two things: to test whether

> a "normal" optab like add_optab is supported for a standard binary

> operation and to test whether a "convert" optab is supported for a

> widening operation like umul_widen_optab.  In the former case from_mode

> and to_mode must be the same, in the latter from_mode must be narrower

> than to_mode.

> 

> For the former case, find_widening_optab_handler_and_mode is only really

> testing the modes that are passed in.  permit_non_widening must be true

> here.

> 

> For the latter case, find_widening_optab_handler_and_mode should only

> really consider new from_modes that are wider than the original

> from_mode and narrower than the original to_mode.  Logically

> permit_non_widening should be false, since widening optabs aren't

> supposed to take operands that are the same width as the destination.

> We get away with permit_non_widening being true because no target

> would/should define a widening .md pattern with matching modes.

> 

> But really, it seems better for expand_binop to handle these two

> cases itself rather than pushing them down.  With that change,

> find_widening_optab_handler_and_mode is only ever called with

> permit_non_widening set to false and is only ever called with

> a "proper" convert optab.  We then no longer need widening_optab_handler,

> we can just use convert_optab_handler directly.

> 

> The patch also passes the instruction code down to expand_binop_directly.

> This should be more efficient and removes an extra call to

> find_widening_optab_handler_and_mode.

> 

> 

> 2017-10-23  Richard Sandiford  <richard.sandiford@linaro.org>

> 	    Alan Hayward  <alan.hayward@arm.com>

> 	    David Sherwood  <david.sherwood@arm.com>

> 

> gcc/

> 	* optabs-query.h (convert_optab_p): New function, split out from...

> 	(convert_optab_handler): ...here.

> 	(widening_optab_handler): Delete.

> 	(find_widening_optab_handler): Remove permit_non_widening parameter.

> 	(find_widening_optab_handler_and_mode): Likewise.  Provide an

> 	override that operates on mode class wrappers.

> 	* optabs-query.c (widening_optab_handler): Delete.

> 	(find_widening_optab_handler_and_mode): Remove permit_non_widening

> 	parameter.  Assert that the two modes are the same class and that

> 	the "from" mode is narrower than the "to" mode.  Use

> 	convert_optab_handler instead of widening_optab_handler.

> 	* expmed.c (expmed_mult_highpart_optab): Use convert_optab_handler

> 	instead of widening_optab_handler.

> 	* expr.c (expand_expr_real_2): Update calls to

> 	find_widening_optab_handler.

> 	* optabs.c (expand_widen_pattern_expr): Likewise.

> 	(expand_binop_directly): Take the insn_code as a parameter.

> 	(expand_binop): Only call find_widening_optab_handler for

> 	conversion optabs; use optab_handler otherwise.  Update calls

> 	to find_widening_optab_handler and expand_binop_directly.

> 	Use convert_optab_handler instead of widening_optab_handler.

> 	* tree-ssa-math-opts.c (convert_mult_to_widen): Update calls to

> 	find_widening_optab_handler and use scalar_mode rather than

> 	machine_mode.

> 	(convert_plusminus_to_widen): Likewise.

OK.
jeff
diff mbox series

Patch

Index: gcc/optabs-query.h
===================================================================
--- gcc/optabs-query.h	2017-09-14 17:04:19.080694343 +0100
+++ gcc/optabs-query.h	2017-10-23 11:43:01.517673716 +0100
@@ -23,6 +23,14 @@  #define GCC_OPTABS_QUERY_H
 #include "insn-opinit.h"
 #include "target.h"
 
+/* Return true if OP is a conversion optab.  */
+
+inline bool
+convert_optab_p (optab op)
+{
+  return op > unknown_optab && op <= LAST_CONV_OPTAB;
+}
+
 /* Return the insn used to implement mode MODE of OP, or CODE_FOR_nothing
    if the target does not have such an insn.  */
 
@@ -43,7 +51,7 @@  convert_optab_handler (convert_optab op,
 		       machine_mode from_mode)
 {
   unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
-  gcc_assert (op > unknown_optab && op <= LAST_CONV_OPTAB);
+  gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
 }
 
@@ -167,12 +175,11 @@  enum insn_code can_float_p (machine_mode
 enum insn_code can_fix_p (machine_mode, machine_mode, int, bool *);
 bool can_conditionally_move_p (machine_mode mode);
 bool can_vec_perm_p (machine_mode, bool, vec_perm_indices *);
-enum insn_code widening_optab_handler (optab, machine_mode, machine_mode);
 /* Find a widening optab even if it doesn't widen as much as we want.  */
-#define find_widening_optab_handler(A,B,C,D) \
-  find_widening_optab_handler_and_mode (A, B, C, D, NULL)
+#define find_widening_optab_handler(A, B, C) \
+  find_widening_optab_handler_and_mode (A, B, C, NULL)
 enum insn_code find_widening_optab_handler_and_mode (optab, machine_mode,
-						     machine_mode, int,
+						     machine_mode,
 						     machine_mode *);
 int can_mult_highpart_p (machine_mode, bool);
 bool can_vec_mask_load_store_p (machine_mode, machine_mode, bool);
@@ -181,4 +188,20 @@  bool can_atomic_exchange_p (machine_mode
 bool can_atomic_load_p (machine_mode);
 bool lshift_cheap_p (bool);
 
+/* Version of find_widening_optab_handler_and_mode that operates on
+   specific mode types.  */
+
+template<typename T>
+inline enum insn_code
+find_widening_optab_handler_and_mode (optab op, const T &to_mode,
+				      const T &from_mode, T *found_mode)
+{
+  machine_mode tmp;
+  enum insn_code icode = find_widening_optab_handler_and_mode
+    (op, machine_mode (to_mode), machine_mode (from_mode), &tmp);
+  if (icode != CODE_FOR_nothing && found_mode)
+    *found_mode = as_a <T> (tmp);
+  return icode;
+}
+
 #endif
Index: gcc/optabs-query.c
===================================================================
--- gcc/optabs-query.c	2017-09-25 13:57:21.028734061 +0100
+++ gcc/optabs-query.c	2017-10-23 11:43:01.517673716 +0100
@@ -401,44 +401,20 @@  can_vec_perm_p (machine_mode mode, bool
   return true;
 }
 
-/* Like optab_handler, but for widening_operations that have a
-   TO_MODE and a FROM_MODE.  */
-
-enum insn_code
-widening_optab_handler (optab op, machine_mode to_mode,
-			machine_mode from_mode)
-{
-  unsigned scode = (op << 16) | to_mode;
-  if (to_mode != from_mode && from_mode != VOIDmode)
-    {
-      /* ??? Why does find_widening_optab_handler_and_mode attempt to
-	 widen things that can't be widened?  E.g. add_optab... */
-      if (op > LAST_CONV_OPTAB)
-	return CODE_FOR_nothing;
-      scode |= from_mode << 8;
-    }
-  return raw_optab_handler (scode);
-}
-
 /* Find a widening optab even if it doesn't widen as much as we want.
    E.g. if from_mode is HImode, and to_mode is DImode, and there is no
-   direct HI->SI insn, then return SI->DI, if that exists.
-   If PERMIT_NON_WIDENING is non-zero then this can be used with
-   non-widening optabs also.  */
+   direct HI->SI insn, then return SI->DI, if that exists.  */
 
 enum insn_code
 find_widening_optab_handler_and_mode (optab op, machine_mode to_mode,
 				      machine_mode from_mode,
-				      int permit_non_widening,
 				      machine_mode *found_mode)
 {
-  for (; (permit_non_widening || from_mode != to_mode)
-	 && GET_MODE_SIZE (from_mode) <= GET_MODE_SIZE (to_mode)
-	 && from_mode != VOIDmode;
-       from_mode = GET_MODE_WIDER_MODE (from_mode).else_void ())
+  gcc_checking_assert (GET_MODE_CLASS (from_mode) == GET_MODE_CLASS (to_mode));
+  gcc_checking_assert (from_mode < to_mode);
+  FOR_EACH_MODE (from_mode, from_mode, to_mode)
     {
-      enum insn_code handler = widening_optab_handler (op, to_mode,
-						       from_mode);
+      enum insn_code handler = convert_optab_handler (op, to_mode, from_mode);
 
       if (handler != CODE_FOR_nothing)
 	{
Index: gcc/expmed.c
===================================================================
--- gcc/expmed.c	2017-10-23 11:42:34.914720660 +0100
+++ gcc/expmed.c	2017-10-23 11:43:01.515743957 +0100
@@ -3701,7 +3701,7 @@  expmed_mult_highpart_optab (scalar_int_m
 
   /* Try widening multiplication.  */
   moptab = unsignedp ? umul_widen_optab : smul_widen_optab;
-  if (widening_optab_handler (moptab, wider_mode, mode) != CODE_FOR_nothing
+  if (convert_optab_handler (moptab, wider_mode, mode) != CODE_FOR_nothing
       && mul_widen_cost (speed, wider_mode) < max_cost)
     {
       tem = expand_binop (wider_mode, moptab, op0, narrow_op1, 0,
@@ -3740,7 +3740,7 @@  expmed_mult_highpart_optab (scalar_int_m
 
   /* Try widening multiplication of opposite signedness, and adjust.  */
   moptab = unsignedp ? smul_widen_optab : umul_widen_optab;
-  if (widening_optab_handler (moptab, wider_mode, mode) != CODE_FOR_nothing
+  if (convert_optab_handler (moptab, wider_mode, mode) != CODE_FOR_nothing
       && size - 1 < BITS_PER_WORD
       && (mul_widen_cost (speed, wider_mode)
 	  + 2 * shift_cost (speed, mode, size-1)
Index: gcc/expr.c
===================================================================
--- gcc/expr.c	2017-10-23 11:42:52.013721093 +0100
+++ gcc/expr.c	2017-10-23 11:43:01.517673716 +0100
@@ -8640,7 +8640,7 @@  #define REDUCE_BIT_FIELD(expr)	(reduce_b
 	{
 	  machine_mode innermode = TYPE_MODE (TREE_TYPE (treeop0));
 	  this_optab = usmul_widen_optab;
-	  if (find_widening_optab_handler (this_optab, mode, innermode, 0)
+	  if (find_widening_optab_handler (this_optab, mode, innermode)
 		!= CODE_FOR_nothing)
 	    {
 	      if (TYPE_UNSIGNED (TREE_TYPE (treeop0)))
@@ -8675,7 +8675,7 @@  #define REDUCE_BIT_FIELD(expr)	(reduce_b
 
 	  if (TREE_CODE (treeop0) != INTEGER_CST)
 	    {
-	      if (find_widening_optab_handler (this_optab, mode, innermode, 0)
+	      if (find_widening_optab_handler (this_optab, mode, innermode)
 		    != CODE_FOR_nothing)
 		{
 		  expand_operands (treeop0, treeop1, NULL_RTX, &op0, &op1,
@@ -8697,7 +8697,7 @@  #define REDUCE_BIT_FIELD(expr)	(reduce_b
 					       unsignedp, this_optab);
 		  return REDUCE_BIT_FIELD (temp);
 		}
-	      if (find_widening_optab_handler (other_optab, mode, innermode, 0)
+	      if (find_widening_optab_handler (other_optab, mode, innermode)
 		    != CODE_FOR_nothing
 		  && innermode == word_mode)
 		{
Index: gcc/optabs.c
===================================================================
--- gcc/optabs.c	2017-10-23 11:42:34.919720660 +0100
+++ gcc/optabs.c	2017-10-23 11:43:01.518638595 +0100
@@ -264,7 +264,7 @@  expand_widen_pattern_expr (sepops ops, r
       || ops->code == WIDEN_MULT_MINUS_EXPR)
     icode = find_widening_optab_handler (widen_pattern_optab,
 					 TYPE_MODE (TREE_TYPE (ops->op2)),
-					 tmode0, 0);
+					 tmode0);
   else
     icode = optab_handler (widen_pattern_optab, tmode0);
   gcc_assert (icode != CODE_FOR_nothing);
@@ -989,17 +989,14 @@  avoid_expensive_constant (machine_mode m
 }
 
 /* Helper function for expand_binop: handle the case where there
-   is an insn that directly implements the indicated operation.
+   is an insn ICODE that directly implements the indicated operation.
    Returns null if this is not possible.  */
 static rtx
-expand_binop_directly (machine_mode mode, optab binoptab,
+expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
 		       rtx op0, rtx op1,
 		       rtx target, int unsignedp, enum optab_methods methods,
 		       rtx_insn *last)
 {
-  machine_mode from_mode = widened_mode (mode, op0, op1);
-  enum insn_code icode = find_widening_optab_handler (binoptab, mode,
-						      from_mode, 1);
   machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
   machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
   machine_mode mode0, mode1, tmp_mode;
@@ -1123,6 +1120,7 @@  expand_binop (machine_mode mode, optab b
     = (methods == OPTAB_LIB || methods == OPTAB_LIB_WIDEN
        ? OPTAB_WIDEN : methods);
   enum mode_class mclass;
+  enum insn_code icode;
   machine_mode wider_mode;
   scalar_int_mode int_mode;
   rtx libfunc;
@@ -1156,23 +1154,30 @@  expand_binop (machine_mode mode, optab b
 
   /* If we can do it with a three-operand insn, do so.  */
 
-  if (methods != OPTAB_MUST_WIDEN
-      && find_widening_optab_handler (binoptab, mode,
-				      widened_mode (mode, op0, op1), 1)
-	    != CODE_FOR_nothing)
+  if (methods != OPTAB_MUST_WIDEN)
     {
-      temp = expand_binop_directly (mode, binoptab, op0, op1, target,
-				    unsignedp, methods, last);
-      if (temp)
-	return temp;
+      if (convert_optab_p (binoptab))
+	{
+	  machine_mode from_mode = widened_mode (mode, op0, op1);
+	  icode = find_widening_optab_handler (binoptab, mode, from_mode);
+	}
+      else
+	icode = optab_handler (binoptab, mode);
+      if (icode != CODE_FOR_nothing)
+	{
+	  temp = expand_binop_directly (icode, mode, binoptab, op0, op1,
+					target, unsignedp, methods, last);
+	  if (temp)
+	    return temp;
+	}
     }
 
   /* If we were trying to rotate, and that didn't work, try rotating
      the other direction before falling back to shifts and bitwise-or.  */
   if (((binoptab == rotl_optab
-	&& optab_handler (rotr_optab, mode) != CODE_FOR_nothing)
+	&& (icode = optab_handler (rotr_optab, mode)) != CODE_FOR_nothing)
        || (binoptab == rotr_optab
-	   && optab_handler (rotl_optab, mode) != CODE_FOR_nothing))
+	   && (icode = optab_handler (rotl_optab, mode)) != CODE_FOR_nothing))
       && is_int_mode (mode, &int_mode))
     {
       optab otheroptab = (binoptab == rotl_optab ? rotr_optab : rotl_optab);
@@ -1188,7 +1193,7 @@  expand_binop (machine_mode mode, optab b
 			       gen_int_mode (bits, GET_MODE (op1)), op1,
 			       NULL_RTX, unsignedp, OPTAB_DIRECT);
 
-      temp = expand_binop_directly (int_mode, otheroptab, op0, newop1,
+      temp = expand_binop_directly (icode, int_mode, otheroptab, op0, newop1,
 				    target, unsignedp, methods, last);
       if (temp)
 	return temp;
@@ -1235,7 +1240,8 @@  expand_binop (machine_mode mode, optab b
       else if (binoptab == rotr_optab)
 	otheroptab = vrotr_optab;
 
-      if (otheroptab && optab_handler (otheroptab, mode) != CODE_FOR_nothing)
+      if (otheroptab
+	  && (icode = optab_handler (otheroptab, mode)) != CODE_FOR_nothing)
 	{
 	  /* The scalar may have been extended to be too wide.  Truncate
 	     it back to the proper size to fit in the broadcast vector.  */
@@ -1249,7 +1255,7 @@  expand_binop (machine_mode mode, optab b
 	  rtx vop1 = expand_vector_broadcast (mode, op1);
 	  if (vop1)
 	    {
-	      temp = expand_binop_directly (mode, otheroptab, op0, vop1,
+	      temp = expand_binop_directly (icode, mode, otheroptab, op0, vop1,
 					    target, unsignedp, methods, last);
 	      if (temp)
 		return temp;
@@ -1272,7 +1278,7 @@  expand_binop (machine_mode mode, optab b
 		&& (find_widening_optab_handler ((unsignedp
 						  ? umul_widen_optab
 						  : smul_widen_optab),
-						 next_mode, mode, 0)
+						 next_mode, mode)
 		    != CODE_FOR_nothing)))
 	  {
 	    rtx xop0 = op0, xop1 = op1;
@@ -1703,7 +1709,7 @@  expand_binop (machine_mode mode, optab b
       && optab_handler (add_optab, word_mode) != CODE_FOR_nothing)
     {
       rtx product = NULL_RTX;
-      if (widening_optab_handler (umul_widen_optab, int_mode, word_mode)
+      if (convert_optab_handler (umul_widen_optab, int_mode, word_mode)
 	  != CODE_FOR_nothing)
 	{
 	  product = expand_doubleword_mult (int_mode, op0, op1, target,
@@ -1713,7 +1719,7 @@  expand_binop (machine_mode mode, optab b
 	}
 
       if (product == NULL_RTX
-	  && (widening_optab_handler (smul_widen_optab, int_mode, word_mode)
+	  && (convert_optab_handler (smul_widen_optab, int_mode, word_mode)
 	      != CODE_FOR_nothing))
 	{
 	  product = expand_doubleword_mult (int_mode, op0, op1, target,
@@ -1806,10 +1812,13 @@  expand_binop (machine_mode mode, optab b
 
   if (CLASS_HAS_WIDER_MODES_P (mclass))
     {
+      /* This code doesn't make sense for conversion optabs, since we
+	 wouldn't then want to extend the operands to be the same size
+	 as the result.  */
+      gcc_assert (!convert_optab_p (binoptab));
       FOR_EACH_WIDER_MODE (wider_mode, mode)
 	{
-	  if (find_widening_optab_handler (binoptab, wider_mode, mode, 1)
-		  != CODE_FOR_nothing
+	  if (optab_handler (binoptab, wider_mode)
 	      || (methods == OPTAB_LIB
 		  && optab_libfunc (binoptab, wider_mode)))
 	    {
Index: gcc/tree-ssa-math-opts.c
===================================================================
--- gcc/tree-ssa-math-opts.c	2017-10-09 11:51:27.664982724 +0100
+++ gcc/tree-ssa-math-opts.c	2017-10-23 11:43:01.519603474 +0100
@@ -3242,7 +3242,7 @@  convert_mult_to_widen (gimple *stmt, gim
 {
   tree lhs, rhs1, rhs2, type, type1, type2;
   enum insn_code handler;
-  machine_mode to_mode, from_mode, actual_mode;
+  scalar_int_mode to_mode, from_mode, actual_mode;
   optab op;
   int actual_precision;
   location_t loc = gimple_location (stmt);
@@ -3269,7 +3269,7 @@  convert_mult_to_widen (gimple *stmt, gim
     op = usmul_widen_optab;
 
   handler = find_widening_optab_handler_and_mode (op, to_mode, from_mode,
-						  0, &actual_mode);
+						  &actual_mode);
 
   if (handler == CODE_FOR_nothing)
     {
@@ -3290,7 +3290,7 @@  convert_mult_to_widen (gimple *stmt, gim
 
 	  op = smul_widen_optab;
 	  handler = find_widening_optab_handler_and_mode (op, to_mode,
-							  from_mode, 0,
+							  from_mode,
 							  &actual_mode);
 
 	  if (handler == CODE_FOR_nothing)
@@ -3350,8 +3350,7 @@  convert_plusminus_to_widen (gimple_stmt_
   optab this_optab;
   enum tree_code wmult_code;
   enum insn_code handler;
-  scalar_mode to_mode, from_mode;
-  machine_mode actual_mode;
+  scalar_mode to_mode, from_mode, actual_mode;
   location_t loc = gimple_location (stmt);
   int actual_precision;
   bool from_unsigned1, from_unsigned2;
@@ -3509,7 +3508,7 @@  convert_plusminus_to_widen (gimple_stmt_
      this transformation is likely to pessimize code.  */
   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);
+						  from_mode, &actual_mode);
 
   if (handler == CODE_FOR_nothing)
     return false;