This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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 <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
Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]