Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
On 11/05/2015 09:01 AM, Ilya Enkovich wrote:
2015-10-27 23:52 GMT+03:00 Jeff Law :
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
Re: [vec-cmp, patch 1/6] Add optabs for vector comparison
2015-10-27 23:52 GMT+03:00 Jeff Law :
>
> 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
On 10/22/2015 10:12 AM, Ilya Enkovich wrote:
2015-10-22 18:52 GMT+03:00 Jeff Law :
On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
2015-10-21 20:25 GMT+03:00 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
* 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
2015-10-22 18:52 GMT+03:00 Jeff Law :
> On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
>>
>> 2015-10-21 20:25 GMT+03:00 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
* 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
On 10/22/2015 04:35 AM, Ilya Enkovich wrote:
2015-10-21 20:25 GMT+03:00 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
* 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
2015-10-21 20:25 GMT+03:00 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
>>
>> * 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
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
* 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
