[vec-cmp, patch 1/6] Add optabs for vector comparison

[vec-cmp, patch 1/6] Add optabs for vector comparison

[Ilya Enkovich]

Hi,

This series introduces autogeneration of vector comparison and its support on i386 target.  It lets comparison statements to be vectorized into vector comparison instead of VEC_COND_EXPR.  This allows to avoid some restrictions implied by boolean patterns.  This series applies on top of bolean vectors series [1].

This patch introduces optabs for vector comparison.

[1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html

Thanks,
Ilya
--
gcc/

2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>

	* expr.c (do_store_flag): Use expand_vec_cmp_expr for mask results.
	* optabs-query.h (get_vec_cmp_icode): New.
	* optabs-tree.c (expand_vec_cmp_expr_p): New.
	* optabs-tree.h (expand_vec_cmp_expr_p): New.
	* optabs.c (vector_compare_rtx): Add OPNO arg.
	(expand_vec_cond_expr): Adjust to vector_compare_rtx change.
	(expand_vec_cmp_expr): New.
	* optabs.def (vec_cmp_optab): New.
	(vec_cmpu_optab): New.
	* optabs.h (expand_vec_cmp_expr): New.
	* tree-vect-generic.c (expand_vector_comparison): Add vector
	comparison optabs check.


diff --git a/gcc/expr.c b/gcc/expr.c
index 0bbfccd..88da8cb 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -11025,9 +11025,15 @@ do_store_flag (sepops ops, rtx target, machine_mode mode)
   if (TREE_CODE (ops->type) == VECTOR_TYPE)
     {
       tree ifexp = build2 (ops->code, ops->type, arg0, arg1);
-      tree if_true = constant_boolean_node (true, ops->type);
-      tree if_false = constant_boolean_node (false, ops->type);
-      return expand_vec_cond_expr (ops->type, ifexp, if_true, if_false, target);
+      if (VECTOR_BOOLEAN_TYPE_P (ops->type))
+	return expand_vec_cmp_expr (ops->type, ifexp, target);
+      else
+	{
+	  tree if_true = constant_boolean_node (true, ops->type);
+	  tree if_false = constant_boolean_node (false, ops->type);
+	  return expand_vec_cond_expr (ops->type, ifexp, if_true,
+				       if_false, target);
+	}
     }
 
   /* Get the rtx comparison code to use.  We know that EXP is a comparison
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 73f2729..81ac362 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -74,6 +74,16 @@ trapv_binoptab_p (optab binoptab)
 	  || binoptab == smulv_optab);
 }
 
+/* Return insn code for a comparison operator with VMODE
+   resultin MASK_MODE, unsigned if UNS is true.  */
+
+static inline enum insn_code
+get_vec_cmp_icode (machine_mode vmode, machine_mode mask_mode, bool uns)
+{
+  optab tab = uns ? vec_cmpu_optab : vec_cmp_optab;
+  return convert_optab_handler (tab, vmode, mask_mode);
+}
+
 /* Return insn code for a conditional operator with a comparison in
    mode CMODE, unsigned if UNS is true, resulting in a value of mode VMODE.  */
 
diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
index 3b03338..aa863cf 100644
--- a/gcc/optabs-tree.c
+++ b/gcc/optabs-tree.c
@@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
   return false;
 }
 
+/* Return TRUE if appropriate vector insn is available
+   for vector comparison expr with vector type VALUE_TYPE
+   and resulting mask with MASK_TYPE.  */
+
+bool
+expand_vec_cmp_expr_p (tree value_type, tree mask_type)
+{
+  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
+					    TYPE_MODE (mask_type),
+					    TYPE_UNSIGNED (value_type));
+  return (icode != CODE_FOR_nothing);
+}
+
 /* Return TRUE iff, appropriate vector insns are available
    for vector cond expr with vector type VALUE_TYPE and a comparison
    with operand vector types in CMP_OP_TYPE.  */
diff --git a/gcc/optabs-tree.h b/gcc/optabs-tree.h
index bf6c9e3..5b966ca 100644
--- a/gcc/optabs-tree.h
+++ b/gcc/optabs-tree.h
@@ -39,6 +39,7 @@ optab optab_for_tree_code (enum tree_code, const_tree, enum optab_subtype);
 optab scalar_reduc_to_vector (optab, const_tree);
 bool supportable_convert_operation (enum tree_code, tree, tree, tree *,
 				    enum tree_code *);
+bool expand_vec_cmp_expr_p (tree, tree);
 bool expand_vec_cond_expr_p (tree, tree);
 void init_tree_optimization_optabs (tree);
 
diff --git a/gcc/optabs.c b/gcc/optabs.c
index 8d9d742..ca1a6e7 100644
--- a/gcc/optabs.c
+++ b/gcc/optabs.c
@@ -5100,11 +5100,13 @@ get_rtx_code (enum tree_code tcode, bool unsignedp)
 }
 
 /* Return comparison rtx for COND. Use UNSIGNEDP to select signed or
-   unsigned operators. Do not generate compare instruction.  */
+   unsigned operators.  OPNO holds an index of the first comparison
+   operand in insn with code ICODE.  Do not generate compare instruction.  */
 
 static rtx
 vector_compare_rtx (enum tree_code tcode, tree t_op0, tree t_op1,
-		    bool unsignedp, enum insn_code icode)
+		    bool unsignedp, enum insn_code icode,
+		    unsigned int opno)
 {
   struct expand_operand ops[2];
   rtx rtx_op0, rtx_op1;
@@ -5130,7 +5132,7 @@ vector_compare_rtx (enum tree_code tcode, tree t_op0, tree t_op1,
 
   create_input_operand (&ops[0], rtx_op0, m0);
   create_input_operand (&ops[1], rtx_op1, m1);
-  if (!maybe_legitimize_operands (icode, 4, 2, ops))
+  if (!maybe_legitimize_operands (icode, opno, 2, ops))
     gcc_unreachable ();
   return gen_rtx_fmt_ee (rcode, VOIDmode, ops[0].value, ops[1].value);
 }
@@ -5386,7 +5388,7 @@ expand_vec_cond_expr (tree vec_cond_type, tree op0, tree op1, tree op2,
   if (icode == CODE_FOR_nothing)
     return 0;
 
-  comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode);
+  comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode, 4);
   rtx_op1 = expand_normal (op1);
   rtx_op2 = expand_normal (op2);
 
@@ -5400,6 +5402,40 @@ expand_vec_cond_expr (tree vec_cond_type, tree op0, tree op1, tree op2,
   return ops[0].value;
 }
 
+/* Generate insns for a vector comparison into a mask.  */
+
+rtx
+expand_vec_cmp_expr (tree type, tree exp, rtx target)
+{
+  struct expand_operand ops[4];
+  enum insn_code icode;
+  rtx comparison;
+  machine_mode mask_mode = TYPE_MODE (type);
+  machine_mode vmode;
+  bool unsignedp;
+  tree op0a, op0b;
+  enum tree_code tcode;
+
+  op0a = TREE_OPERAND (exp, 0);
+  op0b = TREE_OPERAND (exp, 1);
+  tcode = TREE_CODE (exp);
+
+  unsignedp = TYPE_UNSIGNED (TREE_TYPE (op0a));
+  vmode = TYPE_MODE (TREE_TYPE (op0a));
+
+  icode = get_vec_cmp_icode (vmode, mask_mode, unsignedp);
+  if (icode == CODE_FOR_nothing)
+    return 0;
+
+  comparison = vector_compare_rtx (tcode, op0a, op0b, unsignedp, icode, 2);
+  create_output_operand (&ops[0], target, mask_mode);
+  create_fixed_operand (&ops[1], comparison);
+  create_fixed_operand (&ops[2], XEXP (comparison, 0));
+  create_fixed_operand (&ops[3], XEXP (comparison, 1));
+  expand_insn (icode, 4, ops);
+  return ops[0].value;
+}
+
 /* Expand a highpart multiply.  */
 
 rtx
diff --git a/gcc/optabs.def b/gcc/optabs.def
index 888b21c..1f9c1cf 100644
--- a/gcc/optabs.def
+++ b/gcc/optabs.def
@@ -61,6 +61,8 @@ OPTAB_CD(vec_load_lanes_optab, "vec_load_lanes$a$b")
 OPTAB_CD(vec_store_lanes_optab, "vec_store_lanes$a$b")
 OPTAB_CD(vcond_optab, "vcond$a$b")
 OPTAB_CD(vcondu_optab, "vcondu$a$b")
+OPTAB_CD(vec_cmp_optab, "vec_cmp$a$b")
+OPTAB_CD(vec_cmpu_optab, "vec_cmpu$a$b")
 
 OPTAB_NL(add_optab, "add$P$a3", PLUS, "add", '3', gen_int_fp_fixed_libfunc)
 OPTAB_NX(add_optab, "add$F$a3")
diff --git a/gcc/optabs.h b/gcc/optabs.h
index 3f29d1b..e5f2622 100644
--- a/gcc/optabs.h
+++ b/gcc/optabs.h
@@ -300,6 +300,9 @@ extern rtx_insn *gen_cond_trap (enum rtx_code, rtx, rtx, rtx);
 /* Generate code for VEC_PERM_EXPR.  */
 extern rtx expand_vec_perm (machine_mode, rtx, rtx, rtx, rtx);
 
+/* Generate code for vector comparison.  */
+extern rtx expand_vec_cmp_expr (tree, tree, rtx);
+
 /* Generate code for VEC_COND_EXPR.  */
 extern rtx expand_vec_cond_expr (tree, tree, tree, tree, rtx);
 
diff --git a/gcc/tree-vect-generic.c b/gcc/tree-vect-generic.c
index a20b9af..4972964 100644
--- a/gcc/tree-vect-generic.c
+++ b/gcc/tree-vect-generic.c
@@ -347,7 +347,8 @@ expand_vector_comparison (gimple_stmt_iterator *gsi, tree type, tree op0,
                           tree op1, enum tree_code code)
 {
   tree t;
-  if (! expand_vec_cond_expr_p (type, TREE_TYPE (op0)))
+  if (!expand_vec_cmp_expr_p (TREE_TYPE (op0), type)
+      && !expand_vec_cond_expr_p (type, TREE_TYPE (op0)))
     t = expand_vector_piecewise (gsi, do_compare, type,
 				 TREE_TYPE (TREE_TYPE (op0)), op0, op1, code);
   else

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Jeff Law]

On 10/08/2015 08:52 AM, Ilya Enkovich wrote:
> Hi,
>
> This series introduces autogeneration of vector comparison and its support on i386 target.  It lets comparison statements to be vectorized into vector comparison instead of VEC_COND_EXPR.  This allows to avoid some restrictions implied by boolean patterns.  This series applies on top of bolean vectors series [1].
>
> This patch introduces optabs for vector comparison.
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>
> 	* expr.c (do_store_flag): Use expand_vec_cmp_expr for mask results.
> 	* optabs-query.h (get_vec_cmp_icode): New.
> 	* optabs-tree.c (expand_vec_cmp_expr_p): New.
> 	* optabs-tree.h (expand_vec_cmp_expr_p): New.
> 	* optabs.c (vector_compare_rtx): Add OPNO arg.
> 	(expand_vec_cond_expr): Adjust to vector_compare_rtx change.
> 	(expand_vec_cmp_expr): New.
> 	* optabs.def (vec_cmp_optab): New.
> 	(vec_cmpu_optab): New.
> 	* optabs.h (expand_vec_cmp_expr): New.
> 	* tree-vect-generic.c (expand_vector_comparison): Add vector
> 	comparison optabs check.
>
>
> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
> index 3b03338..aa863cf 100644
> --- a/gcc/optabs-tree.c
> +++ b/gcc/optabs-tree.c
> @@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
>     return false;
>   }
>
> +/* Return TRUE if appropriate vector insn is available
> +   for vector comparison expr with vector type VALUE_TYPE
> +   and resulting mask with MASK_TYPE.  */
> +
> +bool
> +expand_vec_cmp_expr_p (tree value_type, tree mask_type)
> +{
> +  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
> +					    TYPE_MODE (mask_type),
> +					    TYPE_UNSIGNED (value_type));
> +  return (icode != CODE_FOR_nothing);
> +}
> +
Nothing inherently wrong with the code, but it seems like it's in the 
wrong place.  Why optabs-tree rather than optabs-query?

I think with that fixed this patch will be ready to go onto the trunk.

jeff

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Ilya Enkovich]

2015-10-21 20:25 GMT+03:00 Jeff Law <law@redhat.com>:
> On 10/08/2015 08:52 AM, Ilya Enkovich wrote:
>>
>> Hi,
>>
>> This series introduces autogeneration of vector comparison and its support
>> on i386 target.  It lets comparison statements to be vectorized into vector
>> comparison instead of VEC_COND_EXPR.  This allows to avoid some restrictions
>> implied by boolean patterns.  This series applies on top of bolean vectors
>> series [1].
>>
>> This patch introduces optabs for vector comparison.
>>
>> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html
>>
>> Thanks,
>> Ilya
>> --
>> gcc/
>>
>> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>
>>         * expr.c (do_store_flag): Use expand_vec_cmp_expr for mask
>> results.
>>         * optabs-query.h (get_vec_cmp_icode): New.
>>         * optabs-tree.c (expand_vec_cmp_expr_p): New.
>>         * optabs-tree.h (expand_vec_cmp_expr_p): New.
>>         * optabs.c (vector_compare_rtx): Add OPNO arg.
>>         (expand_vec_cond_expr): Adjust to vector_compare_rtx change.
>>         (expand_vec_cmp_expr): New.
>>         * optabs.def (vec_cmp_optab): New.
>>         (vec_cmpu_optab): New.
>>         * optabs.h (expand_vec_cmp_expr): New.
>>         * tree-vect-generic.c (expand_vector_comparison): Add vector
>>         comparison optabs check.
>>
>>
>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>> index 3b03338..aa863cf 100644
>> --- a/gcc/optabs-tree.c
>> +++ b/gcc/optabs-tree.c
>> @@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
>>     return false;
>>   }
>>
>> +/* Return TRUE if appropriate vector insn is available
>> +   for vector comparison expr with vector type VALUE_TYPE
>> +   and resulting mask with MASK_TYPE.  */
>> +
>> +bool
>> +expand_vec_cmp_expr_p (tree value_type, tree mask_type)
>> +{
>> +  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
>> +                                           TYPE_MODE (mask_type),
>> +                                           TYPE_UNSIGNED (value_type));
>> +  return (icode != CODE_FOR_nothing);
>> +}
>> +
>
> Nothing inherently wrong with the code, but it seems like it's in the wrong
> place.  Why optabs-tree rather than optabs-query?

Because it uses tree type for arguments. There is no single tree usage
in optabs-query.c. I think expand_vec_cond_expr_p is in optabs-tree
for the same reason.

Thanks,
Ilya

>
> I think with that fixed this patch will be ready to go onto the trunk.
>
> jeff
>

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Jeff Law]

On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
> 2015-10-21 20:25 GMT+03:00 Jeff Law <law@redhat.com>:
>> On 10/08/2015 08:52 AM, Ilya Enkovich wrote:
>>>
>>> Hi,
>>>
>>> This series introduces autogeneration of vector comparison and its support
>>> on i386 target.  It lets comparison statements to be vectorized into vector
>>> comparison instead of VEC_COND_EXPR.  This allows to avoid some restrictions
>>> implied by boolean patterns.  This series applies on top of bolean vectors
>>> series [1].
>>>
>>> This patch introduces optabs for vector comparison.
>>>
>>> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html
>>>
>>> Thanks,
>>> Ilya
>>> --
>>> gcc/
>>>
>>> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>>
>>>          * expr.c (do_store_flag): Use expand_vec_cmp_expr for mask
>>> results.
>>>          * optabs-query.h (get_vec_cmp_icode): New.
>>>          * optabs-tree.c (expand_vec_cmp_expr_p): New.
>>>          * optabs-tree.h (expand_vec_cmp_expr_p): New.
>>>          * optabs.c (vector_compare_rtx): Add OPNO arg.
>>>          (expand_vec_cond_expr): Adjust to vector_compare_rtx change.
>>>          (expand_vec_cmp_expr): New.
>>>          * optabs.def (vec_cmp_optab): New.
>>>          (vec_cmpu_optab): New.
>>>          * optabs.h (expand_vec_cmp_expr): New.
>>>          * tree-vect-generic.c (expand_vector_comparison): Add vector
>>>          comparison optabs check.
>>>
>>>
>>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>>> index 3b03338..aa863cf 100644
>>> --- a/gcc/optabs-tree.c
>>> +++ b/gcc/optabs-tree.c
>>> @@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
>>>      return false;
>>>    }
>>>
>>> +/* Return TRUE if appropriate vector insn is available
>>> +   for vector comparison expr with vector type VALUE_TYPE
>>> +   and resulting mask with MASK_TYPE.  */
>>> +
>>> +bool
>>> +expand_vec_cmp_expr_p (tree value_type, tree mask_type)
>>> +{
>>> +  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
>>> +                                           TYPE_MODE (mask_type),
>>> +                                           TYPE_UNSIGNED (value_type));
>>> +  return (icode != CODE_FOR_nothing);
>>> +}
>>> +
>>
>> Nothing inherently wrong with the code, but it seems like it's in the wrong
>> place.  Why optabs-tree rather than optabs-query?
>
> Because it uses tree type for arguments. There is no single tree usage
> in optabs-query.c. I think expand_vec_cond_expr_p is in optabs-tree
> for the same reason.
Note that expand_vec_cond_expr_p doesn't rely on enum insn code.  Well, 
it relies on enum insn code being an integer and CODE_FOR_nothing always 
having the value zero, which is probably worse.

We should clean both of these up so that:

   1. We don't need enum insn_code in optabs-tree
   2. We don't implicitly rely on CODE_FOR_nothing == 0

It may be as simple as a adding a predicate function to optabs-query 
that returns true/false if there's a suitable icode, then using that 
predicate in optabs-tree.

jeff

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Ilya Enkovich]

2015-10-22 18:52 GMT+03:00 Jeff Law <law@redhat.com>:
> On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
>>
>> 2015-10-21 20:25 GMT+03:00 Jeff Law <law@redhat.com>:
>>>
>>> On 10/08/2015 08:52 AM, Ilya Enkovich wrote:
>>>>
>>>>
>>>> Hi,
>>>>
>>>> This series introduces autogeneration of vector comparison and its
>>>> support
>>>> on i386 target.  It lets comparison statements to be vectorized into
>>>> vector
>>>> comparison instead of VEC_COND_EXPR.  This allows to avoid some
>>>> restrictions
>>>> implied by boolean patterns.  This series applies on top of bolean
>>>> vectors
>>>> series [1].
>>>>
>>>> This patch introduces optabs for vector comparison.
>>>>
>>>> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html
>>>>
>>>> Thanks,
>>>> Ilya
>>>> --
>>>> gcc/
>>>>
>>>> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>>>
>>>>          * expr.c (do_store_flag): Use expand_vec_cmp_expr for mask
>>>> results.
>>>>          * optabs-query.h (get_vec_cmp_icode): New.
>>>>          * optabs-tree.c (expand_vec_cmp_expr_p): New.
>>>>          * optabs-tree.h (expand_vec_cmp_expr_p): New.
>>>>          * optabs.c (vector_compare_rtx): Add OPNO arg.
>>>>          (expand_vec_cond_expr): Adjust to vector_compare_rtx change.
>>>>          (expand_vec_cmp_expr): New.
>>>>          * optabs.def (vec_cmp_optab): New.
>>>>          (vec_cmpu_optab): New.
>>>>          * optabs.h (expand_vec_cmp_expr): New.
>>>>          * tree-vect-generic.c (expand_vector_comparison): Add vector
>>>>          comparison optabs check.
>>>>
>>>>
>>>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>>>> index 3b03338..aa863cf 100644
>>>> --- a/gcc/optabs-tree.c
>>>> +++ b/gcc/optabs-tree.c
>>>> @@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
>>>>      return false;
>>>>    }
>>>>
>>>> +/* Return TRUE if appropriate vector insn is available
>>>> +   for vector comparison expr with vector type VALUE_TYPE
>>>> +   and resulting mask with MASK_TYPE.  */
>>>> +
>>>> +bool
>>>> +expand_vec_cmp_expr_p (tree value_type, tree mask_type)
>>>> +{
>>>> +  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
>>>> +                                           TYPE_MODE (mask_type),
>>>> +                                           TYPE_UNSIGNED (value_type));
>>>> +  return (icode != CODE_FOR_nothing);
>>>> +}
>>>> +
>>>
>>>
>>> Nothing inherently wrong with the code, but it seems like it's in the
>>> wrong
>>> place.  Why optabs-tree rather than optabs-query?
>>
>>
>> Because it uses tree type for arguments. There is no single tree usage
>> in optabs-query.c. I think expand_vec_cond_expr_p is in optabs-tree
>> for the same reason.
>
> Note that expand_vec_cond_expr_p doesn't rely on enum insn code.  Well, it
> relies on enum insn code being an integer and CODE_FOR_nothing always having
> the value zero, which is probably worse.

Actually it also uses CODE_FOR_nothing in comparison:

      || get_vcond_icode (TYPE_MODE (value_type), TYPE_MODE (cmp_op_type),
                          TYPE_UNSIGNED (cmp_op_type)) == CODE_FOR_nothing)

There are also two other instances of CODE_FOR_nothing in
optabs-tree.c. Do you want to get rid of "#include insn-codes.h" in
optabs-tree.c? Will it be really better if we replace it with
"#include optabs-query.h"?

Thanks,
Ilya

>
> We should clean both of these up so that:
>
>   1. We don't need enum insn_code in optabs-tree
>   2. We don't implicitly rely on CODE_FOR_nothing == 0
>
> It may be as simple as a adding a predicate function to optabs-query that
> returns true/false if there's a suitable icode, then using that predicate in
> optabs-tree.
>
> jeff
>
>

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Jeff Law]

On 10/22/2015 10:12 AM, Ilya Enkovich wrote:
> 2015-10-22 18:52 GMT+03:00 Jeff Law <law@redhat.com>:
>> On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
>>>
>>> 2015-10-21 20:25 GMT+03:00 Jeff Law <law@redhat.com>:
>>>>
>>>> On 10/08/2015 08:52 AM, Ilya Enkovich wrote:
>>>>>
>>>>>
>>>>> Hi,
>>>>>
>>>>> This series introduces autogeneration of vector comparison and its
>>>>> support
>>>>> on i386 target.  It lets comparison statements to be vectorized into
>>>>> vector
>>>>> comparison instead of VEC_COND_EXPR.  This allows to avoid some
>>>>> restrictions
>>>>> implied by boolean patterns.  This series applies on top of bolean
>>>>> vectors
>>>>> series [1].
>>>>>
>>>>> This patch introduces optabs for vector comparison.
>>>>>
>>>>> [1] https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00215.html
>>>>>
>>>>> Thanks,
>>>>> Ilya
>>>>> --
>>>>> gcc/
>>>>>
>>>>> 2015-10-08  Ilya Enkovich  <enkovich.gnu@gmail.com>
>>>>>
>>>>>           * expr.c (do_store_flag): Use expand_vec_cmp_expr for mask
>>>>> results.
>>>>>           * optabs-query.h (get_vec_cmp_icode): New.
>>>>>           * optabs-tree.c (expand_vec_cmp_expr_p): New.
>>>>>           * optabs-tree.h (expand_vec_cmp_expr_p): New.
>>>>>           * optabs.c (vector_compare_rtx): Add OPNO arg.
>>>>>           (expand_vec_cond_expr): Adjust to vector_compare_rtx change.
>>>>>           (expand_vec_cmp_expr): New.
>>>>>           * optabs.def (vec_cmp_optab): New.
>>>>>           (vec_cmpu_optab): New.
>>>>>           * optabs.h (expand_vec_cmp_expr): New.
>>>>>           * tree-vect-generic.c (expand_vector_comparison): Add vector
>>>>>           comparison optabs check.
>>>>>
>>>>>
>>>>> diff --git a/gcc/optabs-tree.c b/gcc/optabs-tree.c
>>>>> index 3b03338..aa863cf 100644
>>>>> --- a/gcc/optabs-tree.c
>>>>> +++ b/gcc/optabs-tree.c
>>>>> @@ -320,6 +320,19 @@ supportable_convert_operation (enum tree_code code,
>>>>>       return false;
>>>>>     }
>>>>>
>>>>> +/* Return TRUE if appropriate vector insn is available
>>>>> +   for vector comparison expr with vector type VALUE_TYPE
>>>>> +   and resulting mask with MASK_TYPE.  */
>>>>> +
>>>>> +bool
>>>>> +expand_vec_cmp_expr_p (tree value_type, tree mask_type)
>>>>> +{
>>>>> +  enum insn_code icode = get_vec_cmp_icode (TYPE_MODE (value_type),
>>>>> +                                           TYPE_MODE (mask_type),
>>>>> +                                           TYPE_UNSIGNED (value_type));
>>>>> +  return (icode != CODE_FOR_nothing);
>>>>> +}
>>>>> +
>>>>
>>>>
>>>> Nothing inherently wrong with the code, but it seems like it's in the
>>>> wrong
>>>> place.  Why optabs-tree rather than optabs-query?
>>>
>>>
>>> Because it uses tree type for arguments. There is no single tree usage
>>> in optabs-query.c. I think expand_vec_cond_expr_p is in optabs-tree
>>> for the same reason.
>>
>> Note that expand_vec_cond_expr_p doesn't rely on enum insn code.  Well, it
>> relies on enum insn code being an integer and CODE_FOR_nothing always having
>> the value zero, which is probably worse.
>
> Actually it also uses CODE_FOR_nothing in comparison:
>
>        || get_vcond_icode (TYPE_MODE (value_type), TYPE_MODE (cmp_op_type),
>                            TYPE_UNSIGNED (cmp_op_type)) == CODE_FOR_nothing)
>
> There are also two other instances of CODE_FOR_nothing in
> optabs-tree.c. Do you want to get rid of "#include insn-codes.h" in
> optabs-tree.c? Will it be really better if we replace it with
> "#include optabs-query.h"?
Sigh.  I searched for the enum type, not for CODE_FOR_nothing ;(  My bad.

If it's easy to get rid of, yes.  I believe we've got 3 uses of 
CODE_FOR_nothing.  AFAICT in none of those cases do we care about the 
code other than does it correspond to CODE_FOR_nothing.

Ideally we'd like to have both optabs-query and optabs-tree not know 
about insn codes.  The former is supposed to be IR agnostic, but insn 
codes are part of the RTL IR, so that's a wart.  The latter is supposed 
to be tree specific and thus shouldn't know about the RTL IR either.

I'd settle for getting the wart out of optabs-tree and we can put 
further cleanup of optabs-query in the queue.

To get the wart out of optabs-tree all I think we need is a true boolean 
function that tells us if there's a suitable optab.

It's unfortunate that the routines exported by optabs-query are 
can_{extend,float,fix}_p since those would fairly natural for the 
boolean query we want to make and they're used elsewhere, but not in a 
boolean form.

I think that we ought to rename the existing uses & definition of 
can_XXX_p that are exported by optabs-query.c, then creating new 
can_XXX_p for those uses that just care about the boolean status should 
work.  At that point we remove insn-codes.h from optab-tree.c.

Jeff

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Ilya Enkovich]

2015-10-27 23:52 GMT+03:00 Jeff Law <law@redhat.com>:
>
> Sigh.  I searched for the enum type, not for CODE_FOR_nothing ;(  My bad.
>
> If it's easy to get rid of, yes.  I believe we've got 3 uses of
> CODE_FOR_nothing.  AFAICT in none of those cases do we care about the code
> other than does it correspond to CODE_FOR_nothing.
>
> Ideally we'd like to have both optabs-query and optabs-tree not know about
> insn codes.  The former is supposed to be IR agnostic, but insn codes are
> part of the RTL IR, so that's a wart.  The latter is supposed to be tree
> specific and thus shouldn't know about the RTL IR either.
>
> I'd settle for getting the wart out of optabs-tree and we can put further
> cleanup of optabs-query in the queue.
>
> To get the wart out of optabs-tree all I think we need is a true boolean
> function that tells us if there's a suitable optab.
>
> It's unfortunate that the routines exported by optabs-query are
> can_{extend,float,fix}_p since those would fairly natural for the boolean
> query we want to make and they're used elsewhere, but not in a boolean form.
>
> I think that we ought to rename the existing uses & definition of can_XXX_p
> that are exported by optabs-query.c, then creating new can_XXX_p for those
> uses that just care about the boolean status should work.  At that point we
> remove insn-codes.h from optab-tree.c.

Do you want this refactoring be a part of this patch or series?

Thanks,
Ilya

>
> Jeff

Re: [vec-cmp, patch 1/6] Add optabs for vector comparison

[Jeff Law]

On 11/05/2015 09:01 AM, Ilya Enkovich wrote:
> 2015-10-27 23:52 GMT+03:00 Jeff Law <law@redhat.com>:
>>
>> Sigh.  I searched for the enum type, not for CODE_FOR_nothing ;(  My bad.
>>
>> If it's easy to get rid of, yes.  I believe we've got 3 uses of
>> CODE_FOR_nothing.  AFAICT in none of those cases do we care about the code
>> other than does it correspond to CODE_FOR_nothing.
>>
>> Ideally we'd like to have both optabs-query and optabs-tree not know about
>> insn codes.  The former is supposed to be IR agnostic, but insn codes are
>> part of the RTL IR, so that's a wart.  The latter is supposed to be tree
>> specific and thus shouldn't know about the RTL IR either.
>>
>> I'd settle for getting the wart out of optabs-tree and we can put further
>> cleanup of optabs-query in the queue.
>>
>> To get the wart out of optabs-tree all I think we need is a true boolean
>> function that tells us if there's a suitable optab.
>>
>> It's unfortunate that the routines exported by optabs-query are
>> can_{extend,float,fix}_p since those would fairly natural for the boolean
>> query we want to make and they're used elsewhere, but not in a boolean form.
>>
>> I think that we ought to rename the existing uses & definition of can_XXX_p
>> that are exported by optabs-query.c, then creating new can_XXX_p for those
>> uses that just care about the boolean status should work.  At that point we
>> remove insn-codes.h from optab-tree.c.
>
> Do you want this refactoring be a part of this patch or series?
I would have preferred before, but I think we're at a state where doing 
it after makes more sense.  So let's unblock this patch and I'll make an 
exception for the minor refactoring to go in shortly after stage1 closes.

Jeff