* [vec-cmp, patch 1/6] Add optabs for vector comparison
@ 2015-10-08 14:52 Ilya Enkovich
2015-10-21 17:34 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Enkovich @ 2015-10-08 14:52 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-08 14:52 [vec-cmp, patch 1/6] Add optabs for vector comparison Ilya Enkovich
@ 2015-10-21 17:34 ` Jeff Law
2015-10-22 10:37 ` Ilya Enkovich
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2015-10-21 17:34 UTC (permalink / raw)
To: Ilya Enkovich, gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-21 17:34 ` Jeff Law
@ 2015-10-22 10:37 ` Ilya Enkovich
2015-10-22 15:56 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Enkovich @ 2015-10-22 10:37 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-22 10:37 ` Ilya Enkovich
@ 2015-10-22 15:56 ` Jeff Law
2015-10-22 16:21 ` Ilya Enkovich
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2015-10-22 15:56 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-22 15:56 ` Jeff Law
@ 2015-10-22 16:21 ` Ilya Enkovich
2015-10-27 20:57 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Enkovich @ 2015-10-22 16:21 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
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
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-22 16:21 ` Ilya Enkovich
@ 2015-10-27 20:57 ` Jeff Law
2015-11-05 16:02 ` Ilya Enkovich
0 siblings, 1 reply; 8+ messages in thread
From: Jeff Law @ 2015-10-27 20:57 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-27 20:57 ` Jeff Law
@ 2015-11-05 16:02 ` Ilya Enkovich
2015-11-06 16:15 ` Jeff Law
0 siblings, 1 reply; 8+ messages in thread
From: Ilya Enkovich @ 2015-11-05 16:02 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-11-05 16:02 ` Ilya Enkovich
@ 2015-11-06 16:15 ` Jeff Law
0 siblings, 0 replies; 8+ messages in thread
From: Jeff Law @ 2015-11-06 16:15 UTC (permalink / raw)
To: Ilya Enkovich; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-11-06 16:15 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-10-08 14:52 [vec-cmp, patch 1/6] Add optabs for vector comparison Ilya Enkovich
2015-10-21 17:34 ` Jeff Law
2015-10-22 10:37 ` Ilya Enkovich
2015-10-22 15:56 ` Jeff Law
2015-10-22 16:21 ` Ilya Enkovich
2015-10-27 20:57 ` Jeff Law
2015-11-05 16:02 ` Ilya Enkovich
2015-11-06 16:15 ` Jeff Law
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).